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

General: fix phpcs errors in jetpack.php #14134

Merged
merged 2 commits into from
Nov 27, 2019
Merged

General: fix phpcs errors in jetpack.php #14134

merged 2 commits into from
Nov 27, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Nov 27, 2019

Changes proposed in this Pull Request:

This makes things less readable overall, but avoids having commits fail because of the pre-commit hook. This has been annoying since we implemented the phpcs-changed changes, since you have to --no-verify for simple things like updating the version number on a release branch. This also breaks automated committing like in #13977.
I briefly considered just disabling phpcs for those few lines, but I don't think it would look very good to have a phpcs:disable as the first thing we do in the main plugin file.

Testing instructions:

  • Run vendor/bin/phpcs -w jetpack.php
  • Rejoice

Proposed changelog entry for your changes:

  • N/A

This makes things less readable overall, but avoids having commits fail because of the pre-commit hook.
I briefly considered just disabling phpcs for those few lines, but I don't think it would look very good to have a phpcs:disable as the first thing we do in the main plugin file.
@jeherve jeherve added General [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Janitorial [Pri] Normal labels Nov 27, 2019
@jeherve jeherve added this to the 8.1 milestone Nov 27, 2019
@jeherve jeherve requested a review from a team November 27, 2019 11:29
@jeherve jeherve self-assigned this Nov 27, 2019
@jetpackbot
Copy link

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 073f753

@ChaosExAnima
Copy link
Contributor

Looks good here!

@ChaosExAnima ChaosExAnima 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 27, 2019
@jeherve jeherve merged commit bfd1863 into master Nov 27, 2019
@jeherve jeherve deleted the fix/phpcs-jetpackphp branch November 27, 2019 15:05
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants