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

feat: assignable downstream commits #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GavinPJK
Copy link

Adds the ability to assign a list of users to any downstream PRs generated by updatebot. Can also assign the author of the upstream commit by setting spec.rules.assignAuthorToPullRequests: true within updatebot.yaml, see pkg/cmd/pr/test_data/assignauthor/.jx/updatebot.yaml as an example.

Assumes user(s) can be assigned to downstream dependent repositories. Tested and validated with GitHub.

@jenkins-x-bot
Copy link
Contributor

Hi @GavinPJK. Thanks for your PR.

I'm waiting for a jenkins-x-plugins member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

@jenkins-x-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign jordangoasdoue
You can assign the PR to them by writing /assign @jordangoasdoue in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@GavinPJK
Copy link
Author

/cc @msvticket

@msvticket
Copy link
Contributor

/ok-to-test

@msvticket
Copy link
Contributor

The pr pipeline failed during tests:

 	WARNING: failed to find git URL no .git directory could be found from dir test_data/command
		--- FAIL: TestCreate (0.04s)
		    pr_test.go:55: excluding test assignauthor
		    pr_test.go:74: 
		        	Error Trace:	/workspace/source/pkg/cmd/pr/pr_test.go:74
		        	Error:      	Received unexpected error:
		        	            	failed to set commit details: no .git directory could be found from dir test_data/command
		        	Test:       	TestCreate
		        	Messages:   	failed to run command for test command
		FAIL
		FAIL	github.com/jenkins-x-plugins/jx-updatebot/pkg/cmd/pr	0.085s
		FAIL

@msvticket
Copy link
Contributor

/test lint

Copy link
Contributor

@msvticket msvticket left a comment

Choose a reason for hiding this comment

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

Please change the new methods that are only intended to be used inside the package to be private (i.e. initial letter should be lower case)

Copy link
Contributor

@msvticket msvticket left a comment

Choose a reason for hiding this comment

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

Please split the commit into at least two: one with refactoring and on with the functional change.

@msvticket
Copy link
Contributor

Thank you for the contribution! Please see my review comments.

@jenkins-x-bot
Copy link
Contributor

@GavinPJK: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pr 99605e0 link /test pr
lint 99605e0 link /test lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. I understand the commands that are listed here.

@msvticket
Copy link
Contributor

msvticket commented Dec 18, 2024

Here are the lint failures:

 	        pkg/cmd/pr/pr.go:270:45: hugeParam: rule is heavy (88 bytes); consider passing it by pointer (gocritic)
		func (o *Options) GetSparseCheckoutPatterns(rule v1alpha1.Rule) ([]string, error) {
		                                            ^
		pkg/cmd/pr/pr.go:400:31: hugeParam: rule is heavy (88 bytes); consider passing it by pointer (gocritic)
		func (o *Options) ProcessRule(rule v1alpha1.Rule, index int) error {
		                              ^
		pkg/cmd/pr/pr.go:423:35: hugeParam: rule is heavy (88 bytes); consider passing it by pointer (gocritic)
		func (o *Options) ProcessRuleURLs(rule v1alpha1.Rule, baseBranch string) error {
		                                  ^
		pkg/cmd/pr/pr.go:332:1: paramTypeCombine: func(gitURL string, sha string) (string, error) could be replaced with func(gitURL, sha string) (string, error) (gocritic)
		func (o *Options) FindCommitAuthor(gitURL string, sha string) (string, error) {
		^
		pkg/cmd/pr/pr.go:369:1: paramTypeCombine: func(dir string, commitMessage string, commitTitle string, application string) (string, error) could be replaced with func(dir, commitMessage, commitTitle, application string) (string, error) (gocritic)
		func (o *Options) SetCommitDetails(dir string, commitMessage string, commitTitle string, application string) (string, error) {
		^
		pkg/cmd/pr/pr.go:6: File is not `goimports`-ed (goimports)
			"github.com/jenkins-x/go-scm/scm"
		pkg/cmd/pr/pr_test.go:4: File is not `goimports`-ed (goimports)
			"github.com/jenkins-x/go-scm/scm"
		pkg/cmd/pr/pr.go:416:22: error-strings: error strings should not be capitalized or end with punctuation or a newline (revive)
					return fmt.Errorf("Error: Failed to get sparse checkout patterns for rule #%d, error=%v\n", index, err)
					                  ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants