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.
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
SHIP-0038: Implementable Version #194
SHIP-0038: Implementable Version #194
Changes from 2 commits
c3a315e
b91225c
fbf80dc
a709efa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
In my PoC testing, it made sense to leave the semantic version as
vX.Y
format, and omit the trailing.0
. I added a description to that effect, which shows up in the GitHub UI (per the example YAML below).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.
Fo build, we will get a PR that updates the readme. This PR we either simply manually fix, or we must extend the logic. I guess by default it would modify the readme from the current branch and open the PR against main. This will probably not be mergable.
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.
👍 - I'll add a comment that this workflow will need to be modified to open the PR against the release branch.
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.
Looking at the action doc [1], the
base
parameter can be used to specify the PR base branch, and it defaults to "the branch checked out in the workflow." So you could have some temporary skew between the release branch andmain
if leave it as is (or set main to always reference the "latest" release).[1] https://github.com/marketplace/actions/create-pull-request
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.
Well, the readme update with the new tag, we would want it in main?
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.
Looking at the README today, our install instructions reference the release by explicit version number, and it is published against
main
.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.
I still fear we will need a dedicated branch protection rule and cannot use wildcards in the branch name due to the different matrix values in e2e/integration tests. But, we can simply see what happens and adjust.
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.
As in we would need individual branch protection rules for each branch name? That would be a more significant drawback than I thought.
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.
I assume that for things like go.mod updates, cherry-picks will not work well unless this bot has intelligence (=only picks the go.mod change and then runs
go mod tidy
andgo mod vendor
). So, a "contributor creating a new commit with the same solution just for this branch" would also be allowed.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.
👍 - dependency updates have a much higher likelihood of having merge conflicts when cherrypicking.
I'll clarify that this process should be reserved for bug fixes.
🤔 need to check if Dependabot can be configured to run on the release branches.