Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix Travis errors related with data-l10n-id #11453

Merged
merged 1 commit into from
Oct 11, 2017
Merged

Fix Travis errors related with data-l10n-id #11453

merged 1 commit into from
Oct 11, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Oct 11, 2017

Closes #11454

Fix failures on Travis related with data-l10n-id. See https://travis-ci.org/brave/browser-laptop/jobs/286524924#L3411 for examples

moment was updated from 2.18.1 to 2.19.0 yesterday.
See https://gist.github.com/ichernev/5f3f4eb02761b4f765a0cccf02cec603 for the full changelog.

diff: moment/moment@2.18.1...2.19.0

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 11, 2017

This change looks working: https://travis-ci.org/brave/browser-laptop/jobs/286654321#L3827, https://travis-ci.org/brave/browser-laptop/jobs/286672237#L3490

It seems to me that that we would have to consider to start replacing carets with tildes on package.json for now to accept patch updates only. Minor updates could break builds easily and it will take quite long time (took me a day) to find which package causes that breakage. IMO we should eventually remove semver dependency and update package.json manually (until we get ready for using Greenkeeper #1701 for example ).

If this is worth discussing, I'll open another issue for that.

@codecov-io
Copy link

codecov-io commented Oct 11, 2017

Codecov Report

Merging #11453 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #11453      +/-   ##
==========================================
- Coverage   52.49%   52.48%   -0.02%     
==========================================
  Files         268      268              
  Lines       25224    25229       +5     
  Branches     4025     4026       +1     
==========================================
  Hits        13242    13242              
- Misses      11982    11987       +5
Flag Coverage Δ
#unittest 52.48% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
app/browser/api/ledger.js 31.55% <0%> (-0.1%) ⬇️

@luixxiul
Copy link
Contributor Author

This change is required for master only.

master: https://travis-ci.org/brave/browser-laptop/jobs/286615666#L3387
0.20.x: https://travis-ci.org/brave/browser-laptop/jobs/286620747#L3490
0.19.x: https://travis-ci.org/brave/browser-laptop/jobs/286625961#L3534

so it might be master that has a bug / incompatibility with moment.

@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Oct 11, 2017
@luixxiul luixxiul self-assigned this Oct 11, 2017
@luixxiul
Copy link
Contributor Author

moment was added with 18cf5df#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R97 for #4370.

@luixxiul luixxiul requested a review from ayumi October 11, 2017 18:58
@ayumi
Copy link
Contributor

ayumi commented Oct 11, 2017

do we need to update package-lock.json as well?

@luixxiul
Copy link
Contributor Author

Right, I'm going to do that. Thanks for reminding me.

Fix failures on Travis related with data-l10n-id. See https://travis-ci.org/brave/browser-laptop/jobs/286524924#L3411 for examples

Closes #11454
@luixxiul
Copy link
Contributor Author

It's done.

Copy link
Contributor

@ayumi ayumi left a comment

Choose a reason for hiding this comment

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

🌵

@luixxiul luixxiul merged commit d192494 into brave:master Oct 11, 2017
@luixxiul luixxiul deleted the fix-npm-moment-version branch October 11, 2017 20:11
@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants