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.

Sign up for GitHub

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

Jump to bottom

Update dependency i18n-calypso to v5 #16397

Closed
wants to merge 3 commits into from
Closed

Update dependency i18n-calypso to v5 #16397

wants to merge 3 commits into from

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Jul 4, 2020 •
edited
Loading

This PR contains the following updates:

Package Type Update Change
i18n-calypso dependencies major 4.0.0 -> 5.0.0

Release Notes

Automattic/wp-calypso

v5.0.0

Compare Source

v4.1.0

Compare Source

Renovate configuration

📅 Schedule: "every weekend" in timezone UTC.

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

  • If you want to rebase/retry this PR, check this box

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Janitorial labels Jul 4, 2020
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello renovate[bot]! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D45960-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link

jetpackbot commented Jul 4, 2020 •
edited
Loading

Warnings
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes
⚠️ The Privacy section is missing for this PR. Please specify whether this PR includes any changes to data or privacy.

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16397

Generated by 🚫 dangerJS against 5cc3f18

@renovate renovate bot force-pushed the renovate/i18n-calypso-5.x branch from 38c310c to 1bf83f9 Compare July 11, 2020 00:06
@jeherve jeherve mentioned this pull request Jul 16, 2020
Closed
1 task
@renovate renovate bot force-pushed the renovate/i18n-calypso-5.x branch from 1bf83f9 to c651994 Compare July 17, 2020 20:10
@kraftbj
Copy link
Contributor

kraftbj commented Jul 17, 2020

We can't immediately use this as it drops moment support, which we still use: Automattic/wp-calypso#39030

@kraftbj kraftbj requested review from jeherve and akirk July 17, 2020 22:03
@kraftbj kraftbj self-assigned this Jul 17, 2020
@kraftbj kraftbj added this to the 8.8 milestone Jul 17, 2020
@kraftbj
Copy link
Contributor

kraftbj commented Jul 17, 2020 •
edited
Loading

@akirk and @jeherve calypso-i18n removed moment, so I'm looking to use moment directly and set the locale early.

With the other changes to go with the Core i18n, I'm not fully sure how to confirm this is working as expected or if it is failing :).

Could y'all give it a gander?

Edit: Actually, something isn't right with it. I'll look at it next week, but open to suggestions or ideas.

@kraftbj kraftbj removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Jul 17, 2020
@jeherve jeherve mentioned this pull request Jul 22, 2020
Closed
21 tasks
@jeherve jeherve removed this from the 8.8 milestone Jul 23, 2020
This was referenced Sep 30, 2020
Merged
Merged
@jeherve
Copy link
Member

jeherve commented Oct 1, 2020

I'm looking to use moment directly and set the locale early.

Once the 2 PRs I linked to above get merged, this shouldn't be necessary. We'll then be left with i18n-calypso only be needed for numberFormat. Since numberFormat hasn't been updated in a while, I don't think we need to update i18n-calypso for now. We may want to wait until something like this makes it to Core:
WordPress/gutenberg#22628

We'd then be able to get rid of numberFormat (and thus i18n-calypso) in favor of Core.

jeherve added a commit that referenced this pull request Oct 2, 2020
@jeherve
Admin Page: add custom numberFormat implementation
0212fb4 
This would allow us to get rid of the i18n-calypso package; the only reason we keep this package around today is for its #16397 (comment)

This implementation relies on the browser to do all the work, based on the locale given by the browser or by WordPress.
@jeherve jeherve mentioned this pull request Oct 2, 2020
Merged
@jeherve
Copy link
Member

jeherve commented Oct 2, 2020

We may want to wait until something like this makes it to Core

I opened #17357 to give us an alternative solution today.

jeherve added a commit that referenced this pull request Oct 9, 2020
@jeherve
Admin Page: add custom numberFormat implementation
85e0ae5 
This would allow us to get rid of the i18n-calypso package; the only reason we keep this package around today is for its #16397 (comment)

This implementation relies on the browser to do all the work, based on the locale given by the browser or by WordPress.
jeherve added a commit that referenced this pull request Oct 13, 2020
@jeherve
Admin Page: add custom numberFormat implementation
efee95d 
This would allow us to get rid of the i18n-calypso package; the only reason we keep this package around today is for its #16397 (comment)

This implementation relies on the browser to do all the work, based on the locale given by the browser or by WordPress.
jeherve added a commit that referenced this pull request Oct 19, 2020
@jeherve
Admin Page: remove i18n-calypso dependency
d101bf0 
Fixes #16397

Following the work we've done in #16481, we should not be needing this dependency anymore.
@jeherve jeherve self-assigned this Oct 19, 2020
@jeherve jeherve mentioned this pull request Oct 19, 2020
Merged
jeherve added a commit that referenced this pull request Oct 19, 2020
@jeherve
Admin Page: remove i18n-calypso dependency (#17522)
df25a23 
Fixes #16397

Following the work we've done in #16481, we should not be needing this dependency anymore.
@renovate renovate bot deleted the renovate/i18n-calypso-5.x branch October 19, 2020 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve Awaiting requested review from jeherve

@akirk akirk Awaiting requested review from akirk

Assignees

@kraftbj kraftbj

@jeherve jeherve

Labels
[Status] Blocked / Hold Touches WP.com Files [Type] Janitorial
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@matticbot @jetpackbot @kraftbj @jeherve @renovate-bot