Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Implement publishing workflow for standalone plugins that aren't modules #1000
Implement publishing workflow for standalone plugins that aren't modules #1000
Changes from 4 commits
2841611
2a0c800
df66f19
b6c6255
d6e79c8
e802a71
8a3c690
15b1cc0
a4cf3bf
112c080
32b1c3c
9c4674d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be necessary if the
version
is already present here:performance/plugins/auto-sizes/auto-sizes.php
Line 8 in 36fe0dc
Should this not rather parse the version out of the plugin file? Or we also have it present in the
readme.txt
:performance/plugins/auto-sizes/readme.txt
Line 7 in 36fe0dc
I think we should avoid having this version in three different places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we just store the version in Stable Tag of
readme.txt
then the plugin file could haven.e.x.t
as the version which gets replaced during deployment, or vice-versa.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or the version could be stored in
plugins.json
like you have here, but if so, thenn.e.x.t
should be the version inauto-sizes.php
andreadme.txt
to avoid having to manually keep them in sync.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like the idea of storing the version in a single place only. As mentioned
n.e.x.t
strategy can be used to populate the version on the fly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @westonruter, for bringing this up. The plugin versions were added for the deploy process per #935 (comment). Since there is no build project for
plugins
, in my opinion, we should discuss this first and address it in a follow-up issue. What do you think? cc. @joemcgillThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I saw that comment. Still, not ideal to have the version in triplicate. If you want to address that in another issue, sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open a separate issue for further work: #1006
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent some time looking into how we could modify the release job to avoid the duplication of version numbers here for this, and I'm not sure it's worth the effort for now. Once we finish migrating all of the standalone modules to the plugins folder we can clean this up more easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mukeshpanchal27 @joemcgill I just voiced a similar concern as @westonruter in #935 (comment), and his feedback is precisely why the requirements only mentioned plugin slugs (no need to duplicate the version number).
I'm okay for this to be addressed in a separate issue, but since this PR is actually making the change and the
feature/modules-to-plugins
branch currently uses the correct format, it's a little strange IMO to go ahead with this change just to change it back afterwards.If this PR is blocking other work and therefore would benefit from a merge ASAP, I'm okay to fix this again in a follow up issue, but it should be addressed shortly after this PR, in time for 3.0, as it requires changes to the
plugins.json
file format/spec, which will affect documentation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback.
In a4cf3bf, I have reverted the version-related changes for plugins and instead fetched the version from the
readme.txt
file. I updated the script accordingly. I opted not to use the plugin's main file because different plugins have different main files, such asplugin/load.php
orplugin/plugin.php
.