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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --gpg-sign option on commits #518

Merged
merged 5 commits into from Jun 19, 2021
Merged

Conversation

othmane399
Copy link
Contributor

@othmane399 othmane399 commented Mar 26, 2021

Your checklist for this pull request

馃毃Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable.

Description

This pull request add and option on opts hash to sign your commits using the default key that has been set in the config file

Signed-off-by: othmane399 <othmane.elmassari@doctolib.com>
@othmane399 othmane399 force-pushed the master branch 2 times, most recently from 6816f04 to 7690399 Compare March 26, 2021 17:58
Signed-off-by: othmane399 <othmane.elmassari@doctolib.com>
@othmane399
Copy link
Contributor Author

@jcouball can you review this PR please ? it's about to add --gpg-sign flag on commit method

@othmane399 othmane399 changed the title add --gpg-sign option on commits feat: add --gpg-sign option on commits Mar 26, 2021
Copy link
Member

@jcouball jcouball left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this PR. I have a couple of requests:

  1. I suggest that the key passed to Git::Lib#commit be 'gpg_sign' instead of just 'sign'. I think it is important to keep this close to the git CLI. I know this hasn't been done consistently, but I think it is a good idea.
  2. I suggest, since we are here, allow passing an optional as documented in git-commit. If the value of the key is true don't append the "=#{keyid}" to the argument.

Optional (if you don't do this, I can follow up with a separate PR for it):

  • Add a test so that calling commit builds the command correctly. I'd stub out command for this test.
  • Add documentation to the README.md for this option.

Signed-off-by: othmane399 <othmane.elmassari@doctolib.com>
@othmane399
Copy link
Contributor Author

@jcouball thx for your review. I've done what you have recommended, can you recheck ?

Copy link
Member

@jcouball jcouball left a comment

Choose a reason for hiding this comment

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

Thank you for making the requested changes. I follow up with a PR to:

  • Add a test so that calling commit builds the command correctly. I'd stub out command for this test.
  • Add documentation to the README.md for this option.

I plan on building a new minor release within the next week to get this code into the gem.

@jcouball jcouball merged commit 0cef8ac into ruby-git:master Jun 19, 2021
@jcouball jcouball mentioned this pull request Jul 6, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants