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

Only use two arguments of is_protected_meta #14218

Merged
merged 1 commit into from
Dec 12, 2019
Merged

Only use two arguments of is_protected_meta #14218

merged 1 commit into from
Dec 12, 2019

Conversation

akirk
Copy link
Member

@akirk akirk commented Dec 12, 2019 •
edited
Loading

Fixes #14216

Changes proposed in this Pull Request:

  • Since we don't use the third parameter of the is_protected_meta hook and the documentation can be read as "third paramter is optional," let's just not insist on it being there.

Testing instructions

No instructions since this is a precautious fix that doesn't change any behavior but just doesn't expect an argument that is ignored anyway.

@akirk akirk requested a review from a team as a code owner December 12, 2019 11:02
@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes

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 c27481d

@akirk akirk added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label Dec 12, 2019
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Sharing Post sharing, sharing buttons labels Dec 12, 2019
@jeherve jeherve added this to the 8.1 milestone Dec 12, 2019
jeherve
jeherve approved these changes Dec 12, 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.

This should be good to merge. It's a bit weird that such conflict never came up in the past 7 years, but I guess we were just lucky ¯_(ツ)_/¯

@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 Crew. Label will be renamed soon. labels Dec 12, 2019
@akirk akirk merged commit 0963c91 into master Dec 12, 2019
@akirk akirk deleted the fix/is_protected_meta-optional-argument branch December 12, 2019 12:15
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 12, 2019
@akirk
Copy link
Member Author

akirk commented Dec 12, 2019

Thanks for the review. Indeed, I was thinking the same!

jeherve added a commit that referenced this pull request Dec 13, 2019
@jeherve
Changelog: add #14218
50072b4
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
fff089c 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
zinigor added a commit that referenced this pull request Dec 30, 2019
@jeherve @zinigor
Changelog: 8.1 additions (#14166)
d4a7565 
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@akirk akirk

Labels
[Feature] Sharing Post sharing, sharing buttons [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.1
Development

Successfully merging this pull request may close these issues.

Sharedaddy: Fatal Error when is_protected_meta is only called with two arguments.
4 participants
@akirk @jetpackbot @jeherve @matticbot