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

[FEATURE REQUEST] Support custom git params #14

Closed
gomorizsolt opened this issue Oct 31, 2019 · 3 comments
Closed

[FEATURE REQUEST] Support custom git params #14

gomorizsolt opened this issue Oct 31, 2019 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@gomorizsolt
Copy link
Contributor

Our team uses pre-commit to ensure certain tests pass before committing the changes. However, in some cases, it's unnecessary to run these scripts(e.g. when editing README.md) and so we're able to skip them by appending the --no-verify param to the command.

Example

git commit -m "John Doe" --no-verify

What's your opinion on this feature? Can you see any security issue?

@stefanzweifel
Copy link
Owner

What's your opinion on this feature?

As every project differs, I think it would be a good idea to add an optional input-argument for git-commit-options. Could look like this:

    - name: Commit changed files
      uses: stefanzweifel/[email protected]
      with:
        commit_message: Apply php-cs-fixer changes
        commit_options: '--no-verify --signoff'
        branch: ${{ github.head_ref }}
        file_pattern: src/\*.php
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

This would give consumers of the Action more control.

We can append the options at the end of the git-commit line here.

Can you see any security issue?

No, not really. The Action "just" creates a new commit.
However, if you have some critical checks in your pre-commit-settings and you decided to ignore those with --no-verify and something bad happens … that's a decision the user made 🤷‍♂.

@stefanzweifel stefanzweifel added enhancement New feature or request good first issue Good for newcomers labels Oct 31, 2019
@gomorizsolt
Copy link
Contributor Author

However, if you have some critical checks in your pre-commit-settings and you decided to ignore those with --no-verify and something bad happens … that's a decision the user made 🤷‍♂.

Yes, that's rather sort of a responsibility on the user's side to control the arguments passed into the command.

Thanks for the quick feedback and suggestions, I'll submit a PR soon.

@stefanzweifel
Copy link
Owner

Thanks for the quick feedback and suggestions, I'll submit a PR soon.

Thank you for the great ideas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants