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

Remove openpgp and change CreateCommit signature #2935

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

WillAbides
Copy link
Contributor

closes #2932

In #2932 we discussed removing the dependency on openpgp and replacing Commit.SigningKey with an interface.

While I was working on it, I realized that this should be an option to CreateCommit instead of a Commit field. I was going to just ignore this, but adding an interface field to Commit caused issues with Stringify and the generated string tests. Instead of unwinding all that, I'm doing the easy thing first and adding a CreateCommitOptions struct. I can change approaches if necessary.

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #2935 (fd6be71) into master (b45ef89) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2935   +/-   ##
=======================================
  Coverage   98.17%   98.17%           
=======================================
  Files         143      143           
  Lines       12597    12602    +5     
=======================================
+ Hits        12367    12372    +5     
  Misses        156      156           
  Partials       74       74           
Files Changed Coverage Δ
github/git_commits.go 100.00% <100.00%> (ø)

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Awesome, @WillAbides !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

Comment on lines +156 to +158
opts.Signer = github.MessageSignerFunc(func(w io.Writer, r io.Reader) error {
return openpgp.ArmoredDetachSign(w, key, r, nil)
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL that you can wrap a single func in a type cast that can then be used as a receiver in order to satisfy an interface...
so cool, @WillAbides - thank you!

@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Sep 19, 2023
@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 21, 2023
@gmlewis
Copy link
Collaborator

gmlewis commented Sep 21, 2023

Thank you, @valbeat !
Merging.

@gmlewis gmlewis merged commit aa3fcbe into google:master Sep 21, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transitive dependency (cloudflare/circl) making cross compiling difficult
3 participants