feat: better resilience against listing already included cherry-picks #101
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds resilience to the cherry-pick process by detecting and skipping commits that have already been applied to the current branch. This prevents failures when cherry-picks are re-listed and improves the user experience by clearly indicating which cherry-picks can be safely dropped.
Key Changes:
- Adds ancestor checking before applying cherry-picks to prevent duplicate application
- Provides informative console output for skipped commits
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| eval "declare -A SPACK_CHERRYPICKS_FILES_ARRAY=(${SPACK_CHERRYPICKS_FILES})" | ||
| for hash in ${SPACK_CHERRYPICKS} ; do | ||
| if git -C ${SPACK_ROOT} merge-base --is-ancestor "${hash}" HEAD ; then | ||
| echo "Skipping already-applied ${hash}" |
There was a problem hiding this comment.
The hyphenation in "already-applied" is inconsistent with standard English usage. The past participle "applied" should not be hyphenated when used with "already" as it's not functioning as a compound modifier before a noun.
| echo "Skipping already-applied ${hash}" | |
| echo "Skipping already applied ${hash}" |
| eval "declare -A SPACKPACKAGES_CHERRYPICKS_FILES_ARRAY=(${SPACKPACKAGES_CHERRYPICKS_FILES})" | ||
| for hash in ${SPACKPACKAGES_CHERRYPICKS} ; do | ||
| if git -C ${SPACKPACKAGES_ROOT} merge-base --is-ancestor "${hash}" HEAD ; then | ||
| echo "Skipping already-applied ${hash}" |
There was a problem hiding this comment.
The hyphenation in "already-applied" is inconsistent with standard English usage. The past participle "applied" should not be hyphenated when used with "already" as it's not functioning as a compound modifier before a noun.
| echo "Skipping already-applied ${hash}" | |
| echo "Skipping already applied ${hash}" |
Briefly, what does this PR introduce?
This PR is a quality-of-life improvement in advance of #35. It is always a bit annoying to figure out which of the cherry-picks need to be retained. This PR will print the ones that can be dropped, but more importantly it won't fail on them when cherry-pick wants to apply them again (since they will be skipped).