-
Notifications
You must be signed in to change notification settings - Fork 477
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
Deprecate Collection#size(), add Collection#length #151
Conversation
<3 Any thoughts @fkling? |
@@ -110,6 +110,16 @@ class Collection { | |||
* @return {number} | |||
*/ | |||
size() { | |||
process.stderr.write('Collection#size() is deprecated. Use Collection#length instead'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see much harm supporting both. I would just add the other one and not deprecate this one.
👍 I also think there is no need to add a deprecation warning. Rest looks good to me 😄 |
3b6b71f
to
6978d9d
Compare
Removed deprecation. @fkling how do you want me to handle tests for |
I'm actually OK with as it is. No need to add tests back. |
**what is the change?:** `npm` -> `yarn` **why make this change?:** I personally find it confusing to have 'npm' in the docs when I'm trying to use 'yarn' consistently. **test plan:** Visual inspection
Implements #150 (assuming it is wanted).
I was going to use
util.deprecate
, but cannot do that without using the class fields proposal, so I'm just writing a warning tostderr
atm.EditorConfig
setup in Atom killed some extra whitespace. Happy to revert if requested.