Allow to specify github pullrequest merge method #514
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lemeurherve
reviewed
Feb 7, 2022
lemeurherve
reviewed
Feb 7, 2022
lemeurherve
reviewed
Feb 7, 2022
dduportal
previously approved these changes
Feb 9, 2022
|
I still needs additional testing 😁 |
|
It's now ready to be merged. The last commit fix the issue identified during testing |
Signed-off-by: Olblak <me@olblak.com>
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
Github Api expected mergemethod value to be capital letter Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
dduportal
reviewed
Feb 15, 2022
| @@ -83,6 +91,27 @@ type PullRequest struct { | |||
| remotePullRequest PullRequestApi | |||
| } | |||
|
|
|||
| // isMergeMethodValid ensure that we specified a valid merge method. | |||
| func isMergeMethodValid(method string) (bool, error) { | |||
| if len(method) == 0 || | |||
There was a problem hiding this comment.
Suggested change
| if len(method) == 0 || | |
| if method == "" || |
NITPICK: "Idiomatic" way to check for empty strings: when reading it clear that method is a string (while len(method) is ambiguous: is it a slice or a string or something else ?)
dduportal
reviewed
Feb 15, 2022
| err := p.gh.client.Mutate(context.Background(), &mutation, input, nil) | ||
| // The Github Api expects the merge method to be capital letter and don't allows empty value | ||
| // hence the reason to set input.MergeMethod only if the value is not nil | ||
| if len(p.spec.MergeMethod) > 0 { |
There was a problem hiding this comment.
Suggested change
| if len(p.spec.MergeMethod) > 0 { | |
| if p.spec.MergeMethod != "" { |
NITPICK: "Idiomatic" way to check for empty strings: when reading it clear that p.spec.MergeMethod is a string (while len(p.spec.MergeMethod) is ambiguous: is it a slice or a string or something else ?)
dduportal
reviewed
Feb 15, 2022
| var mutation mutationEnablePullRequestAutoMerge | ||
|
|
||
| input := githubv4.EnablePullRequestAutoMergeInput{ | ||
| PullRequestID: githubv4.String(p.remotePullRequest.ID), | ||
| } | ||
|
|
||
| err := p.gh.client.Mutate(context.Background(), &mutation, input, nil) | ||
| // The Github Api expects the merge method to be capital letter and don't allows empty value |
There was a problem hiding this comment.
Suggested change
| // The Github Api expects the merge method to be capital letter and don't allows empty value | |
| // The Github API expects the merge method to be in upper case and does not allow empty values |
dduportal
reviewed
Feb 15, 2022
| return false, ErrBadMergeMethod | ||
| } | ||
|
|
||
| // Validate ensure that a pullrequest spec contains validate fields |
There was a problem hiding this comment.
Suggested change
| // Validate ensure that a pullrequest spec contains validate fields | |
| // Validate ensures that a pullrequest specification contains validate fields |
dduportal
reviewed
Feb 15, 2022
| @@ -83,6 +91,27 @@ type PullRequest struct { | |||
| remotePullRequest PullRequestApi | |||
| } | |||
|
|
|||
| // isMergeMethodValid ensure that we specified a valid merge method. | |||
There was a problem hiding this comment.
Suggested change
| // isMergeMethodValid ensure that we specified a valid merge method. | |
| // isMergeMethodValid ensures that the specified merge method is valid |
dduportal
approved these changes
Feb 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Olblak me@olblak.com
Allow to specify github pullrequest merge method
Fix #492
Allow to specify a pullrequest merge method by using the key
mergemethodset to ["","squash", "merge","rebase"]Test
To test this pull request, you can run the following commands:
go build -o bin/updatecli # Then test using an updatecli manifest that tries to open a pullrequestAdditional Information
Tradeoff
Potential improvement