wp-cli / dist-archive-command

Create a distribution .zip or .tar.gz based on a plugin or theme's .distignore file
https://developer.wordpress.org/cli/commands/dist-archive/
MIT License
47 stars 23 forks source link

Suggested improvement to command retrieving version number #36

Closed jonathanbossenger closed 1 year ago

jonathanbossenger commented 5 years ago

Summary:

When running this command and using the plugin header example described in the WordPress Plugin handbook, the version number is not found and therefore not appended to the zip archive.

Steps to reproduce:

  1. Create plugin using plugin header format described in the WordPress Plugin handbook - https://developer.wordpress.org/plugins/the-basics/header-requirements/
  2. Run wp dist-archive command to create plugin archive

Because the Version in the header example does not have the trailing asterisk, the version number is not found in the plugin header.

As per this slack conversation around the issue - https://wordpress.slack.com/archives/C02RP4T41/p1547804432280500 @swissspidy suggested reusing/extracting the header parsing functionality used by the i18n command in core @schlessera suggested it might be time to extract that to a get_file_header helper method, or similar.

danielbachhuber commented 5 years ago

get_file_header() would create a dependency on a working WordPress install. Instead, I'd suggest we copy its contents into the dist-archive package.

schlessera commented 5 years ago

I was referring to something like WP_CLI\Utils\get_file_header(), not using the WP version.

danielbachhuber commented 5 years ago

Ah 👍

santilin commented 5 years ago

I am new to wp development and I would like to give a try to solve this. I have been able to setup a development environment (thanks @schlessera) and now I'm trying to reproduce the problem and hopefully make a PR.

santilin commented 5 years ago

I've created a PR: https://github.com/wp-cli/dist-archive-command/pull/37

Please, tell me if it is necessary to assert this here and in case it is not, I beg your perdon :)

schlessera commented 5 years ago

@santilin No need to reference it in here, but rather do it the other way around: Add a mention "Fixes #36" in the description of your pull request (just edit your description and add it to the end). This will show a reference in here, and also automatically close this issue as resolved once your PR is merged.

danielbachhuber commented 1 year ago

🚢