Skip to content

Adds an optional --filename-format=<filename-format> argument#74

Merged
danielbachhuber merged 3 commits intowp-cli:mainfrom
i-am-chitti:filename-format-arg
May 25, 2023
Merged

Adds an optional --filename-format=<filename-format> argument#74
danielbachhuber merged 3 commits intowp-cli:mainfrom
i-am-chitti:filename-format-arg

Conversation

@i-am-chitti
Copy link
Copy Markdown
Contributor

Fixes: #72

This PR addresses the following -

  • Add filename-format assoc arg to customize the name of the archive file name
  • By default, it is in the format {name}.{version}. Examples - {name}-{version} , {name}-myOrg.{version}-dist
  • This parameter will be ignored if
    • a custom filename is provided
    • version does not exist
  • Add feature test cases

Screenshots

image

@i-am-chitti i-am-chitti requested a review from a team as a code owner May 25, 2023 04:50
@swissspidy
Copy link
Copy Markdown
Member

We might want to rename this to filename_format as suggested on the issue, making it consistent with the export command.

The tests look good at first glance, I just don't understand why they are not running here on GitHub Actions 🤔

@i-am-chitti i-am-chitti marked this pull request as draft May 25, 2023 10:11
@i-am-chitti i-am-chitti marked this pull request as ready for review May 25, 2023 10:11
@i-am-chitti
Copy link
Copy Markdown
Contributor Author

i-am-chitti commented May 25, 2023

@swissspidy, In this repo, hypen (-) is being used to separate the words like --create-target-dir or --plugin-dirname here. That's why I kept it consistent with the current package args.

IMO, we should follow the WP_CLI convention that is the underscore. This will require us to update the existing args name.

Let me know if we need this update for the single --filename-format or all existing args or none.

Regarding the tests, the tests workflow under .github is configured to be run on a schedule at some time. So, this may be the reason for not running the tests immediately and it may run at some later point in time.

@swissspidy
Copy link
Copy Markdown
Member

Hmm good point with the other args. Let's leave it as is then.

Regarding the tests, the tests workflow under .github is configured to be run on a schedule at some time. So, this may be the reason for not running the tests immediately and it may run at some later point in time.

It's configured to run on schedule and on push and for all pull requests. So that's 3 separate triggers.

See #68 for an example of an older PR where all the tests ran.

@danielbachhuber do you perhaps have an idea why the tests might not be running here? 🤔

@danielbachhuber
Copy link
Copy Markdown
Member

do you perhaps have an idea why the tests might not be running here?

@swissspidy I think the push event is disabled when the scheduled event is disabled.

I updated the README and all of the builds were triggered.

@danielbachhuber danielbachhuber added the command:dist-archive Related to 'dist-archive' command label May 25, 2023
@danielbachhuber
Copy link
Copy Markdown
Member

Thanks for the PR, @i-am-chitti !

@danielbachhuber danielbachhuber merged commit b1aced0 into wp-cli:main May 25, 2023
@danielbachhuber danielbachhuber added this to the 3.0.0 milestone May 25, 2023
@danielbachhuber danielbachhuber changed the title Add filename-format assoc arg Adds an optional --filename-format=<filename-format> argument May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:dist-archive Related to 'dist-archive' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why is not using the symbol - hyphen to separate the name and version?

3 participants