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

WP 5.3: Use modern wp_timezone #13521

Merged
merged 4 commits into from
Oct 24, 2019
Merged

WP 5.3: Use modern wp_timezone #13521

merged 4 commits into from
Oct 24, 2019

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Sep 23, 2019

Contributes to #13513

Changes proposed in this Pull Request:

  • WP 5.3 will add an unified Date/Time API to avoid the dance between the previous timezone OR GMT offset options defining a site's timezone. In Jetpack, let's use that!
  • Since we still support older versions, we should ship a shim for now.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • n/a

Testing instructions:

  • Run this PR while using the developer.wordpress.com/console against the site. Confirm the dates returned are the same before and after the PR.

Proposed changelog entry for your changes:

  • Code Cleanup: Use new functionality available in WordPress 5.3.

@kraftbj kraftbj added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it General [Status] In Progress labels Sep 23, 2019
@kraftbj kraftbj added this to the 7.9 milestone Sep 23, 2019
@kraftbj kraftbj requested a review from a team September 23, 2019 21:36
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33059-code before merging this PR. Thank you!

@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 23, 2019
@jetpackbot
Copy link

jetpackbot commented Sep 23, 2019 •
edited
Loading

Warnings
⚠️

pre-commit hook was skipped for one or more commits

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 🤖

Generated by 🚫 dangerJS against 11beacb

@matticbot
Copy link
Contributor

kraftbj, Your synced wpcom patch D33059-code has been updated.

@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 24, 2019

@Automattic/poseidon - Before reverting out the change to the Sync package, I wanted to run this by y'all. WP is providing a built-in single-function way to pull out a site's timezone (vs having to juggle the two options).

Sync massages the values in a specific way. Is this something that is possible to update to what WP will natively provide or do we need to maintain the exact form?

tyxla
tyxla previously requested changes Sep 25, 2019
View reviewed changes
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Using wp_timezone_string sounds great, but let's make sure that we're covering the case when the related option is empty.

packages/sync/src/Functions.php Outdated Show resolved Hide resolved
@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Status] In Progress and removed [Status] In Progress [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 27, 2019
@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 27, 2019

Holding on the review while I investigate a WP.com-side unit test failure.

@kraftbj
Re-add cloning to API endpoint
11beacb 
Resolves unit test failure on wp.com.
@matticbot
Copy link
Contributor

kraftbj, Your synced wpcom patch D33059-code has been updated.

@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Sep 27, 2019
@tyxla tyxla dismissed their stale review September 27, 2019 16:25

No longer relevant.

@tyxla tyxla requested a review from a team September 27, 2019 16:25
jeherve
jeherve approved these changes Oct 15, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Oct 15, 2019
@jeherve jeherve added the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 15, 2019
@kraftbj kraftbj merged commit 31efa18 into master Oct 24, 2019
@kraftbj kraftbj deleted the update/date-time branch October 24, 2019 02:40
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 24, 2019
@kraftbj
Copy link
Contributor Author

kraftbj commented Oct 24, 2019

Adding the function to WP.com separately via D34429-code .

@kraftbj
Copy link
Contributor Author

kraftbj commented Oct 24, 2019

r198353-wpcom && r198354-wpcom

@jeherve jeherve added [Status] Has Changelog and removed [Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Status] Needs Changelog labels Oct 24, 2019
jeherve added a commit that referenced this pull request Oct 28, 2019
@jeherve
Changelog: add #13521
837cbd7
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
@jeherve jeherve mentioned this pull request Mar 6, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyxla tyxla tyxla left review comments

@jeherve jeherve jeherve approved these changes

Assignees

@kraftbj kraftbj

Labels
General Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
7.9
Development

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