This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Batch Changes - commit message on stdin #55657
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit messages for batch changes can be arbitrary strings, so passing them inline to the command arguments using `-m` is prone to problems and edge cases. Solving those problems and edge cases is non-optimal, so pivot to using a different way to specify commit messages: from a file (stdin in this case). This allows commit messages to contain arbitrary characters without needing to worry about escaping or edge cases.
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 4ffba4e...06f2163.
|
eseliger
approved these changes
Aug 9, 2023
| return false | ||
| } | ||
| if strings.HasPrefix(arg, "-") { | ||
| fmt.Printf("EVALUATING ARGUMENT %d: %s\n", i, arg) |
There was a problem hiding this comment.
Suggested change
| fmt.Printf("EVALUATING ARGUMENT %d: %s\n", i, arg) |
There was a problem hiding this comment.
I don’t feel super strongly about the other comments, but I think we should address this one in a follow up 🙂
| } | ||
|
|
||
| // I hate state machines, but I hate them less than complicated multi-argument checking | ||
| checkFileInput := false |
There was a problem hiding this comment.
can we get a test for this new added behavior?
| // NOTE: join messages with a blank line in between ("\n\n") | ||
| // because the previous behavior was to use multiple -m arguments, | ||
| // which concatenate with a blank line in between. | ||
| // Gerrit is the only code host that usees multiple messages at the moment |
There was a problem hiding this comment.
Suggested change
| // Gerrit is the only code host that usees multiple messages at the moment | |
| // Gerrit is the only code host that uses multiple messages at the moment |
peterguy
added a commit
that referenced
this pull request
Aug 9, 2023
Implemented suggestions to remove debug code, add test and fix typo
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.1 5.1
# Navigate to the new working tree
cd .worktrees/backport-5.1
# Create a new branch
git switch --create backport-55657-to-5.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d1202d873566cf93c74833d5574340997a818c06
# Push it to GitHub
git push --set-upstream origin backport-55657-to-5.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.1Then, create a pull request where the |
github-actions
bot
added
backports
failed-backport-to-5.1
release-blocker
peterguy
added a commit
that referenced
this pull request
Aug 9, 2023
Commit messages for batch changes can be arbitrary strings, so passing them inline to the command arguments using `-m` is prone to problems and edge cases. Solving those problems and edge cases is non-optimal, so pivot to using a different way to specify commit messages: from a file (stdin in this case). This allows commit messages to contain arbitrary characters without needing to worry about escaping or edge cases. in a batch spec, use a commit message that contains newlines (use `message: |` followed by the message on the next line, indented more than `message`). Check the commit message on the code host after the patch has been published. You can include other characters in the commit message (one edge case we've seen is beginning the commit message with a dash) to ensure they are also handled correctly. (cherry picked from commit d1202d8)
peterguy
added a commit
that referenced
this pull request
Aug 9, 2023
davejrt
pushed a commit
that referenced
this pull request
Aug 9, 2023
Commit messages for batch changes can be arbitrary strings, so passing them inline to the command arguments using `-m` is prone to problems and edge cases. Solving those problems and edge cases is non-optimal, so pivot to using a different way to specify commit messages: from a file (stdin in this case). This allows commit messages to contain arbitrary characters without needing to worry about escaping or edge cases. ## Test plan in a batch spec, use a commit message that contains newlines (use `message: |` followed by the message on the next line, indented more than `message`). Check the commit message on the code host after the patch has been published. You can include other characters in the commit message (one edge case we've seen is beginning the commit message with a dash) to ensure they are also handled correctly.
davejrt
pushed a commit
that referenced
this pull request
Aug 9, 2023
peterguy
added a commit
that referenced
this pull request
Aug 24, 2023
jdpleiness
pushed a commit
that referenced
this pull request
Aug 24, 2023
BolajiOlajide
removed
the
release-blocker
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Commit messages for batch changes can be arbitrary strings, so passing them inline to the command arguments using
-mis prone to problems and edge cases.Solving those problems and edge cases is non-optimal, so pivot to using a different way to specify commit messages: from a file (stdin in this case). This allows commit messages to contain arbitrary characters without needing to worry about escaping or edge cases.
Test plan
in a batch spec, use a commit message that contains newlines (use
message: |followed by the message on the next line, indented more thanmessage). Check the commit message on the code host after the patch has been published.You can include other characters in the commit message (one edge case we've seen is beginning the commit message with a dash) to ensure they are also handled correctly.