Re: [PATCH 1/8] Use %B for Split Subject/Body
Junio C Hamano writes: > The question was about the lossage of the blank line, which does not > seem to be related to what this patch wants to do. Ah, missed that. -# 25 +#25 >>> >>> Why the lossage of a SP? >> >> I think this got fixed later in the series. > > That is not a good excuse to introduce breakages in the first place, no? Oh, I agree. I wasn't making excuses. :) >>> It may make sense to lose these "# num" that will have to be touched >>> every time somebody inserts new test pieces in the middle, as a >>> preparatory step before any of these patches, by the way. That will >>> reduce noise in the patches for real changes. >> >> Yeah, I know, but it makes it really easy to find a test when something >> goes wrong. > > That is what "t-*.sh -i" is for, isn't it? Oh, I didn't know about that! -David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
"郑文辉(Techlive Zheng)" writes: > Though, this patch defintely should be merged, becuase no one expects > his commit message be altered durging the splitting process. Are you saying that after double-checking what was posted? It said something like this below, which does not look like 'definitely should be' to me. diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 920c664..f2b6d4a 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -296,7 +296,12 @@ copy_commit() # We're going to set some environment vars here, so # do it in a subshell to get rid of them safely later debug copy_commit "{$1}" "{$2}" "{$3}" + # Use %B rather than %s%n%n%b to handle the special case of a + # commit that only has a subject line. We don't want to + # introduce a newline after the subject, causing generation of + # a new hash. git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" | +# git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" | ( read GIT_AUTHOR_NAME read GIT_AUTHOR_EMAIL -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
2013/1/1 Junio C Hamano : > "David A. Greene" writes: > >> From: Techlive Zheng >> >> Use %B to format the commit message and body to avoid an extra newline >> if a commit only has a subject line. > > Is this an unconditional improvement, or is it generally an > improvement but for some users it may be a regression? I am > guessing it is the former but am just making sure. This patch will make sure the commits in the result branch by using `git-subtree split` stays intact as they were in the original branch. This patch will break the current existing branch that splitted before this patch, becuase these branches were splitted with the wrongly altered commit messages. Maybe a fallback option should be added to make sure these branches could still be updated. Though, this patch defintely should be merged, becuase no one expects his commit message be altered durging the splitting process. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
2013/1/1 Junio C Hamano : > "David A. Greene" writes: > >> From: Techlive Zheng >> >> Use %B to format the commit message and body to avoid an extra newline >> if a commit only has a subject line. > > Is this an unconditional improvement, or is it generally an > improvement but for some users it may be a regression? I am > guessing it is the former but am just making sure. > >> Author:Techlive Zheng >> >> Signed-off-by: David A. Greene > > Please don't do "Author: " which does not add anything new. That is > what "From: " is for. Instead it needs to be a sign-off. > > Also, is that a real name, I have to wonder? > Hmm, sorry about the confusing. I am a Chinese, I coined that first name a couple years ago when I decided to have a unique name across the web. My real name is "郑文辉" in Chinese,translate to English by its pronucation is "Wenhui Zheng",which means "Zheng" is acturally my real last name. The first name "Wenhui" does not have any meaning in English, so I coined it by "Tech" + "Live", which I interprate it as "Technological Living", thus, "Techlive Zheng" is the name I am currently using online. Here are some links: * Let the code talks. https://github.com/techlivezheng * I cross the great GFW to use twitter. https://twitter.com/techlivezheng * Also search "Techlive Zheng" in Google, the result should be unique to me. So, no doubt, I am a real person, just with kind of an uncommon name. >> Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
gree...@obbligato.org writes: > Ack, of course. I don't know how I missed that. > >>> # 15 >>> test_expect_success 'add main6' ' >>> create main6 && >> >> Why? > > It was in the original testsuite from Avery. I didn't add or remove any > tests when I first integrated git-subtree. The question was about the lossage of the blank line, which does not seem to be related to what this patch wants to do. >>> -# 25 >>> +#25 >> >> Why the lossage of a SP? > > I think this got fixed later in the series. That is not a good excuse to introduce breakages in the first place, no? >> It may make sense to lose these "# num" that will have to be touched >> every time somebody inserts new test pieces in the middle, as a >> preparatory step before any of these patches, by the way. That will >> reduce noise in the patches for real changes. > > Yeah, I know, but it makes it really easy to find a test when something > goes wrong. That is what "t-*.sh -i" is for, isn't it? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
Junio C Hamano writes: > "David A. Greene" writes: > >> Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body > > This needs to say "contrib/subtree" somewhere (applies to all > patches in this series). Ok. Shall I re-send everything? >> From: Techlive Zheng >> >> Use %B to format the commit message and body to avoid an extra newline >> if a commit only has a subject line. >> >> Author:Techlive Zheng > > This needs to be a S-o-b instead; is it a real name, by the way? Ok. No idea about the name but his online presence seems consistent at least. >> +# Save this hash for testing later. >> + >> +subdir_hash=`git rev-parse HEAD` >> + > > We prefer $() over ``; much more readable. Ack, of course. I don't know how I missed that. >> # 15 >> test_expect_success 'add main6' ' >> create main6 && > > Why? It was in the original testsuite from Avery. I didn't add or remove any tests when I first integrated git-subtree. >> @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' ' >> check_equal ''"$(git rev-parse splitbr1)"'' "$spl1" > > Is quoting screwed up around here (and in many other places in this > patch)? What are these no-op '' doing? I assumed they are there to get the double-quotes around the command. I'll see about removing them. >> -# 25 >> +#25 > > Why the lossage of a SP? I think this got fixed later in the series. > It may make sense to lose these "# num" that will have to be touched > every time somebody inserts new test pieces in the middle, as a > preparatory step before any of these patches, by the way. That will > reduce noise in the patches for real changes. Yeah, I know, but it makes it really easy to find a test when something goes wrong. -David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
Junio C Hamano writes: > Also, please be careful about the subject line. I doubt that these > 8 patches will stand out as relating to "contrib/subtree", when mixed > in 200 line output of "git shortlog --no-merges". Ah, ok. I'll be more careful next time. -David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
Junio C Hamano writes: > "David A. Greene" writes: > >> From: Techlive Zheng >> >> Use %B to format the commit message and body to avoid an extra newline >> if a commit only has a subject line. > > Is this an unconditional improvement, or is it generally an > improvement but for some users it may be a regression? I am > guessing it is the former but am just making sure. The former. >> Author:Techlive Zheng >> >> Signed-off-by: David A. Greene > > Please don't do "Author: " which does not add anything new. That is > what "From: " is for. Instead it needs to be a sign-off. Ok. Unfortunately I sent a number of patches like that. Do you want me to re-send them? > Also, is that a real name, I have to wonder? No idea. Not likely, I'd say. -David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
"David A. Greene" writes: > Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body This needs to say "contrib/subtree" somewhere (applies to all patches in this series). > From: Techlive Zheng > > Use %B to format the commit message and body to avoid an extra newline > if a commit only has a subject line. > > Author:Techlive Zheng This needs to be a S-o-b instead; is it a real name, by the way? > Signed-off-by: David A. Greene > --- > contrib/subtree/git-subtree.sh |5 +++ > contrib/subtree/t/t7900-subtree.sh | 73 > ++-- > 2 files changed, 49 insertions(+), 29 deletions(-) > > diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh > index 920c664..f2b6d4a 100755 > --- a/contrib/subtree/git-subtree.sh > +++ b/contrib/subtree/git-subtree.sh > @@ -296,7 +296,12 @@ copy_commit() > # We're going to set some environment vars here, so > # do it in a subshell to get rid of them safely later > debug copy_commit "{$1}" "{$2}" "{$3}" > + # Use %B rather than %s%n%n%b to handle the special case of a > + # commit that only has a subject line. We don't want to > + # introduce a newline after the subject, causing generation of > + # a new hash. > git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' > "$1" | > +#git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" | Is it really replacing %s%n%n%b with %B, or is it still an experiment that is disabled? > ( > read GIT_AUTHOR_NAME > read GIT_AUTHOR_EMAIL > diff --git a/contrib/subtree/t/t7900-subtree.sh > b/contrib/subtree/t/t7900-subtree.sh > index bc2eeb0..93eeb09 100755 > --- a/contrib/subtree/t/t7900-subtree.sh > +++ b/contrib/subtree/t/t7900-subtree.sh > @@ -76,6 +76,10 @@ test_expect_success 'add sub1' ' > git branch -m master subproj > ' > > +# Save this hash for testing later. > + > +subdir_hash=`git rev-parse HEAD` > + We prefer $() over ``; much more readable. > # 3 > test_expect_success 'add sub2' ' > create sub2 && > @@ -155,7 +159,6 @@ test_expect_success 'add main-sub5' ' > create subdir/main-sub5 && > git commit -m "main-sub5" > ' > - > # 15 > test_expect_success 'add main6' ' > create main6 && Why? > @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' ' > check_equal ''"$(git rev-parse splitbr1)"'' "$spl1" Is quoting screwed up around here (and in many other places in this patch)? What are these no-op '' doing? > ' > > -# 25 > +#25 Why the lossage of a SP? It may make sense to lose these "# num" that will have to be touched every time somebody inserts new test pieces in the middle, as a preparatory step before any of these patches, by the way. That will reduce noise in the patches for real changes. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
gree...@obbligato.org writes: > "David A. Greene" writes: > >> From: Techlive Zheng >> >> Use %B to format the commit message and body to avoid an extra newline >> if a commit only has a subject line. > > Wow. So that was a spectacular fail. Sorry about th duplicate patch > e-mails. I have no idea how that happened. > > -David Also, please be careful about the subject line. I doubt that these 8 patches will stand out as relating to "contrib/subtree", when mixed in 200 line output of "git shortlog --no-merges". Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
"David A. Greene" writes: > From: Techlive Zheng > > Use %B to format the commit message and body to avoid an extra newline > if a commit only has a subject line. Is this an unconditional improvement, or is it generally an improvement but for some users it may be a regression? I am guessing it is the former but am just making sure. > Author:Techlive Zheng > > Signed-off-by: David A. Greene Please don't do "Author: " which does not add anything new. That is what "From: " is for. Instead it needs to be a sign-off. Also, is that a real name, I have to wonder? Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] Use %B for Split Subject/Body
"David A. Greene" writes: > From: Techlive Zheng > > Use %B to format the commit message and body to avoid an extra newline > if a commit only has a subject line. Wow. So that was a spectacular fail. Sorry about th duplicate patch e-mails. I have no idea how that happened. -David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html