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

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

Closed i-am-chitti closed 1 year ago

i-am-chitti commented 1 year ago

Fixes: #72

This PR addresses the following -

Screenshots

image

swissspidy commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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