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

Debugger: Modernize and increase timeout #13980

Merged
merged 8 commits into from
Nov 7, 2019
Merged

Debugger: Modernize and increase timeout #13980

merged 8 commits into from
Nov 7, 2019

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Nov 6, 2019

Fixes #12470

Adds a 30-second timeout to the other connection test method in the debug suite, pending actually getting that down to only one test.

Also removes pre-WP 5.2 era and pre-PHP 5.3 things that are no longer needed as soon as WP 5.3 ships (which will be before 8.0, so 8.0 will be bumping things to 5.2).

If we need to hold this until 5.2 as our minimum is official, that's fine.

Changes proposed in this Pull Request:

  • Debugger: Increase timeout for an occasionally slow test and modernize code.

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

  • n/a

Testing instructions:

  • Visit the Debug area via the footer of a Jetpack settings page. Any issues or errors?
  • Run the Site Health tool. Try on a slow site.

Proposed changelog entry for your changes:

  • Debugger: Increase timeout for an occasionally slow test.

@kraftbj kraftbj added [Type] Bug When a feature is broken and / or not performing as intended General [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 6, 2019
@kraftbj kraftbj added this to the 8.0 milestone Nov 6, 2019
@kraftbj kraftbj requested review from jeherve and a team November 6, 2019 22:45
@kraftbj kraftbj self-assigned this Nov 6, 2019
@kraftbj
Copy link
Contributor Author

kraftbj commented Nov 6, 2019

As an aside, I'd like to work this into a package—would be great as a dependent on the connection package, but want to get this improvement in without having it be blocked by packaging.

@jetpackbot
Copy link
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: December 3, 2019.
Scheduled code freeze: November 26, 2019

Generated by 🚫 dangerJS against 0cef1a9

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.

This tests well on my end. 👍 I only have one question.

* @return string The plan slug prepended with "JetpackPlan"
*/
private static function what_jetpack_plan() {
// Specifically not deprecating this function since it modifies the output of the Jetpack_Debug_Data::what_jetpack_plan return.
Copy link
Member

Choose a reason for hiding this comment

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

Is this okay to remove this now? The comment makes me worry. :)

Copy link
Contributor Author

@kraftbj kraftbj Nov 7, 2019

Choose a reason for hiding this comment

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

Yeah, that function provided some of the "debug data" that was part of the appended data when using the contact form that was built into Jetpack and all that debug data was legacy for WP versions prior to 5.2 (the old "Advanced Debug Results" link). The specific format of the text was required for Kayako/Zendesk processing to assign a plan value to a ticket.

For WP 5.2+, this stuff never displayed anywhere and with all private support requests going to the contact form (which uses the API and doesn't parse text anyhow), the plan output isn't needed here nor the legacy format that we needed to maintain.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 7, 2019
@kraftbj kraftbj merged commit fdf482d into master Nov 7, 2019
@kraftbj kraftbj deleted the fix/12470 branch November 7, 2019 20:22
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Nov 7, 2019
jeherve added a commit that referenced this pull request Nov 8, 2019
jeherve added a commit that referenced this pull request Nov 15, 2019
jeherve added a commit that referenced this pull request Nov 25, 2019
* 8.0 Release: running changelog

* Changelog: add #13921

* Changelog: add #13980

* Changelog: add #13905

* Changelog: add #13971

* Changelog: add #13984

* Changelog: add #14009

* Changelog: add #13620

* Remove things that will ship in 7.9.1

* Changelog: add 7.9.1 release (#14044)

* Changelog: add base for 7.9.1 release

* Update release date and post link

* Changelog: add #14066

* Update changelog for 7.9.1

* Changelog: add #13405

* Changelog: add #13841

* Changelog: add #13924

* Changelog: add #13986

* Changelog: add #14010, #14028, #14053, #14055.

* Changelog: add #14054

* Changelog: add #14031

* Changelog: add #14039

* Changelog: add #14050

* Changelog: add #14070

* Changelog: add #14082

* Changelog: add #14084

* Changelog: add #14111

* Changelog: add #13961

* Changelog: add #14047

* Changelog: add #14091

* Changelog: add #14108

* Changelog: add #14121
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugger / Site Health: ensure results match Jetpack.com Debug
4 participants