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

add option to use --signoff with git commit #46

Merged
merged 4 commits into from Jul 29, 2020
Merged

Conversation

pvogt09
Copy link
Contributor

@pvogt09 pvogt09 commented Jul 29, 2020

This PR adds the ability to --signoff the created commit by introducing a new options signoff.

Signed-off-by: pvogt09 <50047961+pvogt09@users.noreply.github.com>
Copy link
Owner

@EndBug EndBug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR, but I have some questions:

  • I've run a couple of tests with your code, and it doesn't seem to commit correctly. Could you link me to the tests you ran so that I can confront them?
  • The new option would only add an argument to the git add command, wouldn't it be easier to just add it in the add option (like src --signoff)? If not, are there any issues in doing so?

I know that the --force parameter has its own option in this action, but I feel like it would be better to just add a field that you can use to put your custom options, if that can't already be achieved by using the add field.

@EndBug EndBug added the type: feature New feature or feature request label Jul 29, 2020
@EndBug EndBug added this to In progress in Main board via automation Jul 29, 2020
@pvogt09
Copy link
Contributor Author

pvogt09 commented Jul 29, 2020

I have tested it by changing the build.yml to use the local version of the action and the result was https://github.com/pvogt09/add-and-commit/actions/runs/187259492/workflow which passed and created a commit with the desired commit message and "Signed-off-by" line. For the PR I deleted these commits again and force pushed a more tidyer version. I just recreated the local test in https://github.com/pvogt09/add-and-commit/tree/test.

The aim of the signoff flag is not to change git add (that does not have a --signoff argument btw), it is supposed to add "Signed-off-by: author < email-address >" to the commit message (like in 0cbe3c6). It might be possible to archieve the same with the message option, but then author and email of the push/pull_request that started the workflow have to be known in advance and can not be changed/dynamically derived.

@EndBug
Copy link
Owner

EndBug commented Jul 29, 2020

Sorry, my bad:

  • When I meant that it wasn't committing correctly I meant that it didn't do it correctly when the option was not being used.
  • You're right, that goes to the git commit command, using a dedicated option is ok

Copy link
Owner

@EndBug EndBug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem was caused by the fact that you used double quotes with the sign off variable.
I've also refactored the code to make it look like the add function.

You're welcome to give it a check, if that works for you I'll publish it in the next minor version ;)

Main board automation moved this from In progress to Reviewer approved Jul 29, 2020
@pvogt09
Copy link
Contributor Author

pvogt09 commented Jul 29, 2020

I am ok with the changes. Thank you for the help.

@EndBug EndBug merged commit 51fc21c into EndBug:master Jul 29, 2020
Main board automation moved this from Reviewer approved to Done Jul 29, 2020
@EndBug
Copy link
Owner

EndBug commented Jul 29, 2020

@all-contributors please add @pvogt09 for code

@allcontributors
Copy link
Contributor

@EndBug

I've put up a pull request to add @pvogt09! 🎉

@EndBug
Copy link
Owner

EndBug commented Jul 29, 2020

Ok, you can find the new feature in version v4.3.0 (available also using v4 and latest)

Thanks again for your contribution ✨

EndBug added a commit that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or feature request
Projects
No open projects
Main board
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants