Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document alternatives to deprecated methods #1002

Closed
benjamingr opened this issue Feb 28, 2015 · 8 comments
Closed

Document alternatives to deprecated methods #1002

benjamingr opened this issue Feb 28, 2015 · 8 comments

Comments

@benjamingr
Copy link
Member

Currently the fs.exists method is deprecated (yay for io for doing this) however the documentation does not explain why it was deprecated (which is reasonable) or what to use instead (which is less desirable).

I thought about making a docs PR suggesting using fs.stat or fs.access but I'n not sure which to recommend or if recommending either is appropriate.

Should an alternative be suggested inside the docs and if so which?

@vkurchatkin
Copy link
Contributor

IMHO we should just deprecate exists programmatically and remove it from docs

@othiym23
Copy link
Contributor

@vkurchatkin That's hostile to API users. If I come to the docs looking for exists because I'm trying to remember the details of its use, I'm going to be confused and frustrated when I can't find it. I think the deprecation should be clearly labeled in the docs, and yeah, I think an alternative should be suggested. A single sentence saying "In most cases, fs.stat will give you what you need, but if all you care about is the accessibility of the file, fs.access is useful," wouldn't bulk up the documentation unduly.

@vkurchatkin
Copy link
Contributor

Deprecated methods only exist so that legacy code could work. If you write new code, you should use something else. If you write code for older versions of platform you should use appropriate docs. That's the idea. Maybe a bit hostile, yeah. But still, docs for deprecated methods should be removed EVENTUALLY, but before methods themselves.

@othiym23
Copy link
Contributor

I disagree. I actually think that the documentation should persist for a while after the command has been removed, saying, "this was here, but we deprecated it in semver major version x, we removed it in release semver major version x + n, and so now it's gone." And then perhaps remove the docs in semver major version x + n + 1. This is how Java did it, and it was almost invariably helpful, given how fast the Java APIs moved in its early life.

@benjamingr
Copy link
Member Author

@vkurchatkin the problem is that can, and will break a lot of existing packages. I'm all for removing fs.exists (I even suggested it here in an issue before 1.0 iirc) but even deprecated methods need documentation for people running legacy code. In bluebird there is a deprecated_apis.md file documenting those methods but suggesting that users do not rely on them as they can be removed at any moment. I think exists should definitely go at some point but I don't think we're there yet.

@othiym23 I've created a PR clarifying the usage - can you take a look and let me know if this is what you had in mind?

@vkurchatkin
Copy link
Contributor

@othiym23 it is maybe works for Java because deprecated methods aren't supposed to be removed ever

@benjamingr I'm not talking about removing it anytime soon. Just showing deprecation message

@othiym23
Copy link
Contributor

@vkurchatkin Early on (0.8 - 1.3, roughly), methods and classes got removed all the time, and in fact Java used to move entire APIs around in the namespace tree, sometimes without a lot of warning. It's "easier" with Java because those kinds of rearrangements will cause failures at compile / build time, but creating affordances for this stuff is important if you don't want to aggravate your user community unnecessarily.

@benjamingr
Copy link
Member Author

@vkurchatkin like usually - I agree with your opinion here. Showing a deprecation message can be a huge win for .exists. That said fs.exists is still very widely used - if there was a way to only show the deprecation notice when you're working on the package and not on a project that includes the package as a first step that'd be really cool - bombarding users of popular packages they have no control over with messages will likely be very counterproductive.

tellnes pushed a commit that referenced this issue Mar 1, 2015
At the moment users who want to use `fs.exists` get a warning that the
method is deprecated but do not get offered an alternative in the page.
This PR suggests `fs.stat` and `fs.access` as alternatives while
keeping the warning about the use case in place.

Fixes: #1002
PR-URL: #1007
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Christian Tellnes <[email protected]>
@tellnes tellnes closed this as completed Mar 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants