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

Allowing moment.set() to accept a hash of units #1809

Closed
wants to merge 2 commits into from

Conversation

jordaaash
Copy link
Contributor

Setting multiple values using the chaining API is awkward and verbose.

Currently:

moment().set('year', 2011).set('month', 9).set('date', 12).set('hours', 6).set('minutes', 7).set('seconds', 8);

After this patch, the current API is supported, as well as using a hash of values:

moment().set({ year: 2011, month: 9, date: 12, hours: 6, minutes: 7, seconds: 8 });

PR includes a passing test for the new behavior.

@ichernev
Copy link
Contributor

ichernev commented Aug 5, 2014

I think we need to enforce an order for setting the properties: from bigger units to smaller.

Also we need a few more tests that cover passing {} and null.

@ichernev ichernev added this to the 2.9 milestone Aug 5, 2014
@jordaaash
Copy link
Contributor Author

Got it. I'm not very familiar with moment's internals. What's the purpose of the ordering?
Also, I'm guessing it should noop on {} and return this. Should it also noop on null, should it error, or should it do something else (like a "clear" function)?

@ichernev
Copy link
Contributor

ichernev commented Aug 6, 2014

Nooping is fine. Ordering is for more consistent and less surprising results.

@ichernev
Copy link
Contributor

Merged in 81a3b7b

@ichernev ichernev closed this Dec 24, 2014
ichernev added a commit that referenced this pull request Dec 28, 2014
Allowing moment.set() to accept a hash of units
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

Successfully merging this pull request may close these issues.

None yet

2 participants