-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 /test logic to rerun button for presubmits #13815
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I like inventing a policy when someone does not pass config flag instead of just enforcing that they pass it
a66ed6b
to
639d603
Compare
I changed it so now it skips the /test checks unless the config flag is provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold
LGTM label has been added. Git tree hash: 838a8ee5292e9f598b31f8c820d99d41aa8703ae
|
logrus.WithError(err).Fatal("Error loading Prow plugin config.") | ||
} | ||
} else { | ||
logrus.Warning("No plugins configuration was provided to deck. You must provide one to reuse /test checks for rerun") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change it to info in a follow up
|
||
// If the job is a presubmit and has an associated PR, do the same checks as for /test | ||
if pj.Spec.Type == prowapi.PresubmitJob && pj.Spec.Refs != nil && len(pj.Spec.Refs.Pulls) > 0 { | ||
if pluginAgent == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could add && pluginAgent != nil
to the above and get the same effect, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes... I'll also change this in a followup since I want to get this merged today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ Fixes made in #13865
LGTM label has been added. Git tree hash: 5f991e7092a08847f097f9b023e5506c148db5e9
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta, mirandachrist The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
More work related to #13321. This replicates the checks we do when determining whether a user can use /test on presubmits.
/assign @cjwagner
/cc @fejta