Skip to content

prowgen: generate stricter branch: stanzas for presubmits#2136

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
petr-muller:stricter-feature-branches
Jul 8, 2021
Merged

prowgen: generate stricter branch: stanzas for presubmits#2136
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
petr-muller:stricter-feature-branches

Conversation

@petr-muller
Copy link
Copy Markdown
Member

In the past, we considered any $branch$suffix a feature branch of
"base" CI branch (e.g. PRs to master-next would receive presubmits
from master config). We need to make this convention stricter to not
break on 4.10, because the current convention means that
release-4.10 is a feature branch for release-4.1.

All generated presubmits now have two items in their branches: stanza.
One item that matches exactly the branch (^branch$), just like
postsubmits have, and one item that matches branches with $base-
prefix. Practically this means that we enforce exactly the - separator
for a branch to be considered a feature branch.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2021
@petr-muller
Copy link
Copy Markdown
Member Author

/hold
Needs a o/release followup after merge

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 1, 2021
In the past, we considered any `$branch$suffix` a feature branch of
"base" CI branch (e.g. PRs to `master-next` would receive presubmits
from `master` config). We need to make this convention stricter to not
break on `4.10`, because the current convention means that
`release-4.10` is a feature branch for `release-4.1`.

All generated presubmits now have two items in their `branches:` stanza.
One item that matches exactly the branch (`^branch$`), just like
postsubmits have, and one item that matches branches with `$base-`
prefix. Practically this means that we enforce exactly the `-` separator
for a branch to be considered a feature branch.
@petr-muller petr-muller force-pushed the stricter-feature-branches branch from ff8047a to 560ec91 Compare July 1, 2021 13:17
Copy link
Copy Markdown
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

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

Can we get a unit test for this?

@petr-muller
Copy link
Copy Markdown
Member Author

petr-muller commented Jul 1, 2021

Can we get a unit test for this?

@stevekuznetsov hm, do you have a specific one in mind? GeneratePresubmitForTest is already unit tested (TestGeneratePresubmitForTest) and I just needed to change its fixtures to reflect the changed expectations about its output. Do you think there's any specific case that we would need to test?

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 7, 2021
Copy link
Copy Markdown
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

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

I was thinking something at a lower level than the full fixture-based thing but whatever

/lgtm
/hold cancel

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 7, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, stevekuznetsov

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [petr-muller,stevekuznetsov]

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

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2021
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 8, 2021

@petr-muller: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/breaking-changes 560ec91 link /test breaking-changes

Full PR test history. Your PR dashboard.

Details

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 kubernetes/test-infra repository. I understand the commands that are listed here.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants