Add git stamp #585
Add git stamp #585
Conversation
| issue BAR-123 | ||
| ``` | ||
|
|
||
|
|
There was a problem hiding this comment.
In my opinion, it will be greater if you also show the change of last commit via git log in this introduction.
There was a problem hiding this comment.
You mean showing the exact result of the commands as it they were executed?
Meaning with the commit hash, author name and date as well
bin/git-stamp
Outdated
|
|
||
| error() { | ||
| if [[ -n "$1" ]]; then | ||
| local msg=$( echo "error: $1" | sed 's/\\n/\\n /g' ) |
There was a problem hiding this comment.
What is the reason to add some trailing whitespace after error message?
There was a problem hiding this comment.
Ah that a copy paste from my previous git reauthor command
https://github.com/tardypad/git-extras/blob/stamp/bin/git-reauthor#L32
This was for multiple lines error message so that the second line is aligned with the : as in
error: missing target of the rewrite
use either --old-email option or --all flag
It's not needed here, I better remove it then I guess
| echo "${commit_msg}" \ | ||
| | grep --ignore-case --invert-match "^${ID}\b" \ | ||
| | cat --squeeze-blank | ||
| ) |
There was a problem hiding this comment.
Here is a corner case:
What will happen if commit message without stamp shares same prefix with $ID?
For example, run git stamp 'stamp:' blahblah on your branch.
There was a problem hiding this comment.
Maybe this case is unavoidable... Warn it in the document?
There was a problem hiding this comment.
Indeed this is "unavoidable" the way it is
That's why I show the new commit message as a result, so that people can check that nothing went wrong.
But it's better to be explicit and I'll put a warning in the documentation
| _git_stamp(){ | ||
| __gitcomp '--replace -r' | ||
| } | ||
|
|
There was a problem hiding this comment.
Well, I notice that you have updated the bash completion script. It will be great if you also update the zsh one.
There was a problem hiding this comment.
Alright I'll try to add it since that should be a simple one
I am not sure I'll be able to test it though
Configuring and switching to ZSH is still on my TODO list :)
spacewander
left a comment
spacewander
left a comment
There was a problem hiding this comment.
That is all, my review is finished.
tardypad
left a comment
tardypad
left a comment
There was a problem hiding this comment.
Thanks
Just waiting for your confirmation about the meaning of your comment about the Commands.md file
and then I'll implement the changes
| issue BAR-123 | ||
| ``` | ||
|
|
||
|
|
There was a problem hiding this comment.
You mean showing the exact result of the commands as it they were executed?
Meaning with the commit hash, author name and date as well
bin/git-stamp
Outdated
|
|
||
| error() { | ||
| if [[ -n "$1" ]]; then | ||
| local msg=$( echo "error: $1" | sed 's/\\n/\\n /g' ) |
There was a problem hiding this comment.
Ah that a copy paste from my previous git reauthor command
https://github.com/tardypad/git-extras/blob/stamp/bin/git-reauthor#L32
This was for multiple lines error message so that the second line is aligned with the : as in
error: missing target of the rewrite
use either --old-email option or --all flag
It's not needed here, I better remove it then I guess
| echo "${commit_msg}" \ | ||
| | grep --ignore-case --invert-match "^${ID}\b" \ | ||
| | cat --squeeze-blank | ||
| ) |
There was a problem hiding this comment.
Indeed this is "unavoidable" the way it is
That's why I show the new commit message as a result, so that people can check that nothing went wrong.
But it's better to be explicit and I'll put a warning in the documentation
| _git_stamp(){ | ||
| __gitcomp '--replace -r' | ||
| } | ||
|
|
There was a problem hiding this comment.
Alright I'll try to add it since that should be a simple one
I am not sure I'll be able to test it though
Configuring and switching to ZSH is still on my TODO list :)
|
Waiting for your update... |
|
Every review comment should be fixed within those last commits |
|
@spacewander Any final thoughts? |
|
@tardypad |
replace all previous stamps with same identifier in commit message
no need to put quotes in case of spaces
|
@spacewander I've rebased my branch against master yesterday |
|
Now I received the notification via email, I think it is time to merge the
pr. :)
2016-12-05 17:32 GMT+08:00 Damien Tardy-Panis <[email protected]>:
… @spacewander <https://github.com/spacewander> I've rebased my branch
against master yesterday
I'm not sure if you receive a notification in that case 🤔
So here is this message
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#585 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AD-AbBfSbm74VUBEREwe9XhQYvbjd0u0ks5rE9oUgaJpZM4KP6OP>
.
|
|
Just introduced a new command to |
3 participants
Continuing the moving of some of my local git aliases and functions
in order to make them more maintainable and so that other people can use them too 😁
This adds a new stamp command: Stamp the last commit message
See the manual for the use cases and examples
The command is best used afterwards with personal aliases such as
issue = stamp -r IssueThe usage is then
git issue FOO-142to stamp the commit with an issue number