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

Execute Jetpack::init only once #19167

Merged
merged 2 commits into from
Mar 17, 2021
Merged

Execute Jetpack::init only once #19167

merged 2 commits into from
Mar 17, 2021

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Mar 14, 2021

Already executed in line number 86.

Fixes #19166
Fixes #5684

All is explained in the above issue.

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

  • Try using different parts of Jetpack depending on init.
  • Try updating from the stable version of Jetpack to a new version with this patch, and ensure that the upgrade routine works well.

@jeherve jeherve added [Pri] Normal [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it General labels Mar 15, 2021
@jeherve
Copy link
Member

jeherve commented Mar 15, 2021

I've closed the issue you opened since we've discussed this in the past in #5684. See the links there for more details about the things we've done with this in the past.

Since this is an important part of Jetpack, could you add some testing instructions to your PR, as well as a changelog entry?

You will also need to add a changelog entry, as explained here, so the tests can pass:
https://github.com/Automattic/jetpack/blob/master/docs/writing-a-good-changelog-entry.md
https://github.com/Automattic/jetpack/blob/master/docs/monorepo.md#using-the-jetpack-changelogger

Thank you!

@szepeviktor
Copy link
Contributor Author

Since this is an important part of Jetpack, could you add some testing instructions to your PR, as well as a changelog entry?

Changelog entry added.
I have zero clue what test could help here.

@jeherve jeherve added this to the jetpack/9.6 milestone Mar 17, 2021
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Mar 17, 2021
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.

I've tested this, it seems to work well. When updating, the upgrade routine still runs well, and no issues came up after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible hook timing calling Jetpack init more then once during load.
3 participants