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

Bump moment and remove locale tests #505

Merged
merged 2 commits into from
May 11, 2017
Merged

Bump moment and remove locale tests #505

merged 2 commits into from
May 11, 2017

Conversation

majapw
Copy link
Collaborator

@majapw majapw commented May 11, 2017

Fix for #497

We were pegging moment to a lower version because a number of our date conversion tests were testing for format explicitly and moment frequently updates locale formats in minor and patch releases. This updates moment and removes the offending tests.

The tests are a bit of a relic of when this repository was internal to Airbnb. At that time, the formatted string values were being passed around and had to be in a specific format or face breaking pages. Now that there's a bit of healthy separation (the input elements themselves use ISO format and the callbacks from the components all pass around moment objects instead of strings) the locale tests don't do as much for us within the repo itself. :) As such, just knowing the toLocalizedDateString, toMomentObject, and toISODateString all handle the L format appropriately is enough.

to: @ljharb @jasonkb @airbnb/webinfra

@majapw majapw added the semver-patch: fixes/refactors/etc Anything that's not major or minor. label May 11, 2017
ljharb
ljharb previously approved these changes May 11, 2017
@coveralls
Copy link

coveralls commented May 11, 2017

Coverage Status

Coverage remained the same at 86.526% when pulling b6650af on maja-fix-locale-tests into 61d6249 on master.

@ljharb ljharb added semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep. and removed semver-patch: fixes/refactors/etc Anything that's not major or minor. labels May 11, 2017
@@ -113,7 +113,7 @@
"react-portal": "^3.0.0"
},
"peerDependencies": {
"moment": "2.10 - 2.14 || ~2.15.1 || ~2.16 || ~2.17",
"moment": "^2.18.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be a patch, this needs to be 2.10 - 2.14 || ~2.15.1 || ~2.16 || ~2.17 || ^2.18.1

@majapw majapw merged commit a6d295b into master May 11, 2017
@majapw majapw deleted the maja-fix-locale-tests branch May 11, 2017 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major: breaking change A non-backwards-compatible change; anything that breaks code - including adding a peerDep.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants