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

update "requires at least" value to 5.6 #29646

Merged
merged 3 commits into from
Mar 15, 2021
Merged

Conversation

bph
Copy link
Contributor

@bph bph commented Mar 8, 2021

of plugin file.

Description

For plugin, the minimum requirement is 5.5 - it's already updated in the readme.txt but not in the plugin header of gutenberg.php

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@bph bph changed the title update requires at least line update "requires at least" value to 5.5 Mar 8, 2021
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

It's 5.6 now :) - #29701

@gziolo
Copy link
Member

gziolo commented Mar 10, 2021

I think it should be 5.6 now after @youknowriad merged #29701 😄

@gziolo gziolo added the Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts label Mar 10, 2021
gutenberg.php Outdated
@@ -3,7 +3,7 @@
* Plugin Name: Gutenberg
* Plugin URI: https://github.com/WordPress/gutenberg
* Description: Printing since 1440. This is the development plugin for the new block editor in core.
* Requires at least: 5.3
* Requires at least: 5.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can remove this entirely from here? I mean we already have this value defined in readme.txt isn't that enough?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep them. See https://core.trac.wordpress.org/ticket/46938

Copy link
Member

Choose a reason for hiding this comment

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

Could add Tested up to here too, fwiw

Copy link
Member

Choose a reason for hiding this comment

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

See the last comment from @SergeyBiryukov:

If the headers are defined in both readme.txt and the main plugin file, precedence is given to the plugin file.

Copy link
Member

@SergeyBiryukov SergeyBiryukov Mar 10, 2021

Choose a reason for hiding this comment

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

Do you think we can remove this entirely from here? I mean we already have this value defined in readme.txt isn't that enough?

Just noting that WordPress 5.8 aims to remove parsing readme.txt files from validate_plugin_requirements(), see https://core.trac.wordpress.org/ticket/48520. Per discussion with Otto, apparently that file should only have been used for the Plugin Directory and not for core, which should retrieve the data it needs from the main plugin file instead.

So I would suggest removing the Requires at least and Requires PHP headers from the readme.txt file, and keeping them only in the main plugin file. The Plugin Directory supports that too, see https://meta.trac.wordpress.org/ticket/4514.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying. I will make sure we update @wordpress/create-block and its templates to follow the same approach.

@bph
Copy link
Contributor Author

bph commented Mar 11, 2021

Wasn't fast enough 🤣😂 @youknowriad took care of it. 👍🦄Thanks for all the comments.

@bph bph closed this Mar 11, 2021
@youknowriad
Copy link
Contributor

@bph we actually stiil need to do that if you want to give it a try. I wasn't aware of the flags in the php file personally #29646 (comment)

@bph
Copy link
Contributor Author

bph commented Mar 11, 2021

Oh, sorry. Yes, I can update to 5.6. Hang-on

@bph bph reopened this Mar 11, 2021
@bph bph changed the title update "requires at least" value to 5.5 update "requires at least" value to 5.6 Mar 11, 2021
@bph bph requested a review from youknowriad March 11, 2021 07:59
@youknowriad
Copy link
Contributor

youknowriad commented Mar 11, 2021

@bph can you also remove the two headers from the readme.txt to avoid duplication?

@bph
Copy link
Contributor Author

bph commented Mar 11, 2021

Updated @youknowriad -

@youknowriad
Copy link
Contributor

Looks like this PR needs a rebase now (conflict with the readme update on trunk). So you can do something like git fetch origin && git rebase origin trunk and follow the instructions.

Copy link
Contributor Author

@bph bph left a comment

Choose a reason for hiding this comment

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

Should be fixed now @youknowriad -- it still shows a conflict?

@youknowriad
Copy link
Contributor

@bph I can take care of it.

@youknowriad
Copy link
Contributor

Looks we have some issues with wp-env at the moment, I'll merge this after we solve these.

@youknowriad youknowriad merged commit aac145e into WordPress:trunk Mar 15, 2021
@github-actions github-actions bot added this to the Gutenberg 10.3 milestone Mar 15, 2021
@bph bph deleted the patch-1 branch March 31, 2021 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants