-
Notifications
You must be signed in to change notification settings - Fork 43
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
CASSANDRA-18399: Add a script to prepare merge commands #89
base: trunk
Are you sure you want to change the base?
CASSANDRA-18399: Add a script to prepare merge commands #89
Conversation
dev/prepare-merge-commands.sh
Outdated
|
||
set -e | ||
|
||
supported_versions=("3.0" "3.11" "4.0" "4.1" "trunk") |
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.
nit: adds another file to the codebase the release manager has to manually update when we 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.
ok, I think this is not needed at all. I can assume we start from 3.0 and go through cassandra-x.y branches from the repository up to trunk. All in all the user is responsible to providing the branch they start from.
dev/prepare-merge-commands.sh
Outdated
echo "git add CHANGES.txt" | ||
echo "git commit --amend --no-edit" | ||
|
||
echo "(git diff $apache_repository/${target_branches[$i]}..HEAD -- .circleci/ | git apply -R --index) && git commit -a --amend --no-edit # Remove all changes in .circleci directory if you need to" |
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.
nit: i'm a bit uneasy about this. i think it's simpler and safer if we just expect the user to remove the throwaway circleci commits before calling this script. (though we should add a safety catch that fails if such a commit exists.)
: if this was used for CASSANDRA-18153 it demonstrates a few failures:
|
My apologize for screwing things up. It was not caused by the script but by myself - I misunderstood how the CHANGES.txt should look like and I thought that the same rules apply for all the unreleased branches. I manually updated CHANGES.txt for 4.1 and for trunk, and apparently I forgot to add The script updates CHANGES.txt in the way that for the first release branch the changes is for it adds an entry right after the version. For all other release branches, it searches for the first occurence of "Merged from x.y" where x.y is the first branch version the changes is for. Actually, that CHANGES.txt update "logic" can be extracted into a separate script so we can implement it in say Python or something more robust than Bash. Note that, the script is not aiming to change something, replace or so, just provide some aid and make merging stuff a bit easier and faster. The problems spotted here shows that it lacks some verification for git history. |
np @jacek-lewandowski , it's very trivial, and excuse me when i'm terse, that's only ever efficiency never tone. |
Agree. Also, what I always do is a dry-run push.
and then double-check both diff and show outputs before doing the real push.
Not necessarily for this PR, but can you imagine making these verification steps easier/included too? |
No description provided.