Re: [PATCH] branch: use $curr_branch_short more
Am 09.09.2013 01:13, schrieb Felipe Contreras: On Sun, Sep 8, 2013 at 10:21 AM, René Scharfe l@web.de wrote: Am 31.08.2013 19:20, schrieb Felipe Contreras: A summary should contain as much information that would allow me to skip the commit message as possible. If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job. trivial simplification explains the what, and the why at the same time, and allows most people to skip the commit message, thus is a good summary. No patch should be skipped on the mailing list. As you wrote, trivial patches can still be wrong. What patches each persons skips is each person's own decision, don't be patronizing, if I want to skip a trivial patch, I will, I can't read each and every patch from the dozens of mailing lists I'm subscribed to, and there's no way each and every reader is going to read each and every patch. They should be prioritized, and trivial ones can be safely skipped by most people. Yes, of course; someone needs to review every patch in the end, but each reader decides for themselves which ones to skip. I can't keep up with the traffic either. By the way, the bikeshedding phenomenon probably causes trivial patches to receive the most attention. :) When going through the history I can see that quickly recognizing insubstantial changes is useful, but if I see a summary twice then in my mind forms a big question mark -- why did the same thing had to be done yet again? As an example, both 0d12e59f (pull: replace unnecessary sed invocation) and bc2bbc45 (pull, rebase: simplify to use die()) could arguably have had the summary trivial simplification, but I'm glad the author went with something a bit more specific. Well I wont. Because it takes long to read, and after reading I still don't don't if they are trivial or not, I might actually have to read the commit message, but to be honest, I would probably go right into the diff itself, because judging from Git's history, chances are that somebody wrote a novel there with the important bit I'm looking for just at the end, to don't ruin the suspense. Ha! It's better to write it down at all than to miss it years later, when even the original author has forgotten all about it. In the first commit, it's saying it's a single invocation, so I take it it's trivial, but what is it replaced with? Is the code simpler, is it more complex? I don't know, I'm still not being told *why* that patch is made. It says 'unnecessary' but why is it unnecessary? The sed call is unnecessary because of the fact that it can be replaced. :) I'm sure I would have understood it to mean a conversion to Shell builtin functionality in order to avoid forking and executing an external command, even if I hadn't read the patch. In the second commit, it's saying it's a simplification, but I don't know if it's just one instance, or a thousand, so I don't know what would be the impact of the patch. Either way I'm forced to read more just to know if it was safe for me to skip them, at which point the whole purpose of a summary is defeated. I don't see how using trivial simplification as the summary for both could have improved matters. Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness. Well, in terms of impact I agree. No, in all terms. A patch can be: Trivial and correct Trivial and incorrect Non-trivial and correct Non-trivial and incorrect Well, yes, but I thought your statement that The patch is trivial was referring to my actual patch which started this sub-thread. And I meant that the benefit of that patch to users and programmers was small. To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance. That's one definition; I think I had the mathematical notion in mind that calls proofs trivial which are immediately evident. We are not talking about mathematics, we are talking about the English human language. Here we were talking about source code patches. As formal descriptions of changes to (mostly) programming language text they are closer to mathematics than English. Using math terms when talking about them is not too far of a stretch. René -- 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] branch: use $curr_branch_short more
On Sun, Sep 15, 2013 at 6:42 AM, René Scharfe l@web.de wrote: Am 09.09.2013 01:13, schrieb Felipe Contreras: On Sun, Sep 8, 2013 at 10:21 AM, René Scharfe l@web.de wrote: Am 31.08.2013 19:20, schrieb Felipe Contreras: A summary should contain as much information that would allow me to skip the commit message as possible. If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job. trivial simplification explains the what, and the why at the same time, and allows most people to skip the commit message, thus is a good summary. No patch should be skipped on the mailing list. As you wrote, trivial patches can still be wrong. What patches each persons skips is each person's own decision, don't be patronizing, if I want to skip a trivial patch, I will, I can't read each and every patch from the dozens of mailing lists I'm subscribed to, and there's no way each and every reader is going to read each and every patch. They should be prioritized, and trivial ones can be safely skipped by most people. Yes, of course; someone needs to review every patch in the end, but each reader decides for themselves which ones to skip. I can't keep up with the traffic either. By the way, the bikeshedding phenomenon probably causes trivial patches to receive the most attention. :) Exactly, so if the summary of the commit message allows people to skip a patch, that is fine. When going through the history I can see that quickly recognizing insubstantial changes is useful, but if I see a summary twice then in my mind forms a big question mark -- why did the same thing had to be done yet again? As an example, both 0d12e59f (pull: replace unnecessary sed invocation) and bc2bbc45 (pull, rebase: simplify to use die()) could arguably have had the summary trivial simplification, but I'm glad the author went with something a bit more specific. Well I wont. Because it takes long to read, and after reading I still don't don't if they are trivial or not, I might actually have to read the commit message, but to be honest, I would probably go right into the diff itself, because judging from Git's history, chances are that somebody wrote a novel there with the important bit I'm looking for just at the end, to don't ruin the suspense. Ha! It's better to write it down at all than to miss it years later, when even the original author has forgotten all about it. Yes, of course, but that still means the commit message summary failed it's purpose. In the first commit, it's saying it's a single invocation, so I take it it's trivial, but what is it replaced with? Is the code simpler, is it more complex? I don't know, I'm still not being told *why* that patch is made. It says 'unnecessary' but why is it unnecessary? The sed call is unnecessary because of the fact that it can be replaced. :) I'm sure I would have understood it to mean a conversion to Shell builtin functionality in order to avoid forking and executing an external command, even if I hadn't read the patch. The problem is that the commit message is not for you, it's for every reader, so the fact that you would have understood it that way is irrelevant. Maybe this is an exercise in the lack of empathy, and an example of mono-culture. In the second commit, it's saying it's a simplification, but I don't know if it's just one instance, or a thousand, so I don't know what would be the impact of the patch. Either way I'm forced to read more just to know if it was safe for me to skip them, at which point the whole purpose of a summary is defeated. I don't see how using trivial simplification as the summary for both could have improved matters. It would say trivial, which allows me and a lot of other people to safely skip them, it's as simple as that. Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness. Well, in terms of impact I agree. No, in all terms. A patch can be: Trivial and correct Trivial and incorrect Non-trivial and correct Non-trivial and incorrect Well, yes, but I thought your statement that The patch is trivial was referring to my actual patch which started this sub-thread. And I meant that the benefit of that patch to users and programmers was small. I don't understand what you are trying to say, the point remains; a patch being trivial says nothing about its correctness. To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance. That's one definition; I think I had the mathematical notion in mind that calls proofs trivial which are immediately evident. We are not talking about mathematics, we are talking about the English human language. Here we were talking about source code
Re: [PATCH] branch: use $curr_branch_short more
Am 31.08.2013 19:20, schrieb Felipe Contreras: A summary should contain as much information that would allow me to skip the commit message as possible. If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job. trivial simplification explains the what, and the why at the same time, and allows most people to skip the commit message, thus is a good summary. No patch should be skipped on the mailing list. As you wrote, trivial patches can still be wrong. When going through the history I can see that quickly recognizing insubstantial changes is useful, but if I see a summary twice then in my mind forms a big question mark -- why did the same thing had to be done yet again? As an example, both 0d12e59f (pull: replace unnecessary sed invocation) and bc2bbc45 (pull, rebase: simplify to use die()) could arguably have had the summary trivial simplification, but I'm glad the author went with something a bit more specific. I agree that some kind of tagging with keywords like trivial, typo and so on can be helpful, though. Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness. Well, in terms of impact I agree. To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance. That's one definition; I think I had the mathematical notion in mind that calls proofs trivial which are immediately evident. René -- 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] branch: use $curr_branch_short more
On Sun, Sep 8, 2013 at 10:21 AM, René Scharfe l@web.de wrote: Am 31.08.2013 19:20, schrieb Felipe Contreras: A summary should contain as much information that would allow me to skip the commit message as possible. If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job. trivial simplification explains the what, and the why at the same time, and allows most people to skip the commit message, thus is a good summary. No patch should be skipped on the mailing list. As you wrote, trivial patches can still be wrong. What patches each persons skips is each person's own decision, don't be patronizing, if I want to skip a trivial patch, I will, I can't read each and every patch from the dozens of mailing lists I'm subscribed to, and there's no way each and every reader is going to read each and every patch. They should be prioritized, and trivial ones can be safely skipped by most people. Here's a good example from a simple summary that I didn't write: t2024: Fix inconsequential typos http://article.gmane.org/gmane.comp.version-control.git/234038 Do I have to read this patch? No. I know it's inconsequential, I can safely skip it, the key word being *I*. *Somebody* should read it, sure, and I'm sure at least Junio would, but I don't need to. When going through the history I can see that quickly recognizing insubstantial changes is useful, but if I see a summary twice then in my mind forms a big question mark -- why did the same thing had to be done yet again? As an example, both 0d12e59f (pull: replace unnecessary sed invocation) and bc2bbc45 (pull, rebase: simplify to use die()) could arguably have had the summary trivial simplification, but I'm glad the author went with something a bit more specific. Well I wont. Because it takes long to read, and after reading I still don't don't if they are trivial or not, I might actually have to read the commit message, but to be honest, I would probably go right into the diff itself, because judging from Git's history, chances are that somebody wrote a novel there with the important bit I'm looking for just at the end, to don't ruin the suspense. In the first commit, it's saying it's a single invocation, so I take it it's trivial, but what is it replaced with? Is the code simpler, is it more complex? I don't know, I'm still not being told *why* that patch is made. It says 'unnecessary' but why is it unnecessary? In the second commit, it's saying it's a simplification, but I don't know if it's just one instance, or a thousand, so I don't know what would be the impact of the patch. Either way I'm forced to read more just to know if it was safe for me to skip them, at which point the whole purpose of a summary is defeated. Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness. Well, in terms of impact I agree. No, in all terms. A patch can be: Trivial and correct Trivial and incorrect Non-trivial and correct Non-trivial and incorrect To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance. That's one definition; I think I had the mathematical notion in mind that calls proofs trivial which are immediately evident. We are not talking about mathematics, we are talking about the English human language. -- Felipe Contreras -- 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] branch: use $curr_branch_short more
Felipe Contreras felipe.contre...@gmail.com writes: Subject: branch: use $curr_branch_short more Why? I don't think that summary explains the reason for being for this patch, also, it starts with branch: instead of pull: Subject: pull: trivial simplification With that summary, people would have an easier time figuring out if they need to read more about the patch or not. People, other than the author of the patch, use the subject line in different ways. It is unclear to me which usage the trivial lets them decide if it needs further reading is trying to help. 1. Those running shortlog to see how much impact each contributor had. 2. Those viewing list of Subjects in their MUA to see which is worth reading and commenting on. 3. Those viewing list of Subjects in their MUA to see which is worth reading and applying to their trees. 4. Those trying to resolve conflicts during git am -3 and git merge [*1*]. 5. Those who find that a commit broke the build after bisecting, and want to see what is the reasoning behind the broken change. 6. Those who find an interesting section of the code, blamed its origin to a commit, and want to see what is the reasoning behind the broken change. There may be others, but the above should cover most of the cases, I think. For 1., they may discount anything that says trivial as not of high impact. There may be trivial but high impact changes, but I do not know how much this mischaracterization hurts. Probably not that much. For 2., they may either skip trivial, thinking if it is trivial, it must be correct, or read trivial, thinking the author thinks trivial, it is likely there are holes in the author's thinking. Among the 6 patches in $gmane/233472 Trivial cleanups and fixes, - 1, 2, 4 and 5 were trivially correct and good, - 3 was not a clear improvement, - 6 was wrong. This is totally unscientific but if we take them as a representative set of trivial patches, a trivial patch is correct only about 4.5/6 = 75% of the time. As a consequence, people who do want to help the project are better off reading trivial just like others, so that breakages in the 25% do not go unnoticed. The label does not let them skip, and wastes one line that possibly could have given them more information with non-descriptive word trivial. For 3., unless the author wants such a patch skipped and dropped on the floor, marking it 'trivial' to allow them skip would not make much sense, and with 75% success rate, it would be irresponsible for those who apply patches not to read a trivial and blindly apply it. Labelling it trivial only wastes one line that possibly could have given them more information with non-descriptive word trivial. For 4., 5., and 6., allow them to skip will not be in the picture, as they already know they are interested in that particular change. Labelling it trivial only wastes one line that possibly could have given them more information with non-descriptive word trivial So it seems to me that a title trivial only helps those running shortlog to discount weight of contributor's contribution. And the author who does not want to spend time on coming up with a good title, but for each patch, there are a lot more readers than a single author of that patch, so the benefit to the author does not count much. [Footnote] *1* The former shows the title of the patch and the latter shows the branch name after a conflict marker, and it helps to have as much clue as possible to remind what is attempted on each side of the change. It is a responsibility for the person who does the merge to give the branch a descriptive name, and the branch that queued a trivial patch does not have to be named trivial, but the title of the patch is a large part of the input to naming the branch. -- 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] branch: use $curr_branch_short more
Subject: branch: use $curr_branch_short more Why? I don't think that summary explains the reason for being for this patch, also, it starts with branch: instead of pull: Subject: pull: trivial simplification With that summary, people would have an easier time figuring out if they need to read more about the patch or not. -- Felipe Contreras -- 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] branch: use $curr_branch_short more
Am 31.08.2013 10:22, schrieb Felipe Contreras: Subject: branch: use $curr_branch_short more Why? I don't think that summary explains the reason for being for this patch, also, it starts with branch: instead of pull: You're right about branch vs. pull. I'll better go back to bed. ~_~ Subject: pull: trivial simplification With that summary, people would have an easier time figuring out if they need to read more about the patch or not. trivial simplification is too generic; we could have lots of them. A summary should describe the change. Its low complexity can be derived from it -- using an existing variable a bit more is not very exciting. But I wouldn't call that patch trivial because its correctness depends on code outside of its shown context. The reason for the patch isn't mentioned explicitly. Perhaps it should be. I felt that using something that's already there instead of recreating it is motivation alone. René -- 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] branch: use $curr_branch_short more
On Sat, Aug 31, 2013 at 5:28 AM, René Scharfe l@web.de wrote: Am 31.08.2013 11:22, schrieb Felipe Contreras: On Sat, Aug 31, 2013 at 4:11 AM, René Scharfe l@web.de wrote: Subject: pull: trivial simplification With that summary, people would have an easier time figuring out if they need to read more about the patch or not. trivial simplification is too generic; we could have lots of them. No, we can have only one, otherwise it would say simplificationS. I was too terse again, let me rephrase that: We could have lots of commits that fit the same description if we used such a generic one. Yes, but they are all trivial, and they all simply the code. A summary should describe the change. You can never fully describe the change, only the diff does that. For example use $curr_branch_short more does not tell me anything about the extent of the changes, is it used in one more place? two? one hundred? Moreover, how exactly is it used more? Is some refactoring needed? And it still doesn't answer the most important question any summary should answer: why? Why use $curr_branch_short more? A summary doesn't have to contain lots of details. The what is important, the why can be explained in the commit message. A summary should contain as much information that would allow me to skip the commit message as possible. If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job. trivial simplification explains the what, and the why at the same time, and allows most people to skip the commit message, thus is a good summary. Its low complexity can be derived from it -- using an existing variable a bit more is not very exciting. You didn't say a bit more you said more. And yes, the complexity can be derived from the summary, but not from this one. But I wouldn't call that patch trivial because its correctness depends on code outside of its shown context. Correctness is a separate question from triviality, and the correctness can only be assessed by looking at the actual patch. The patch can be both trivial and wrong. Probably too terse again, let's say it differently: Only a patch whose correctness can be judged without looking outside of the three lines of context it includes qualifies as trivial in my book. The patch in question is not trivial because you can't see the value of $curr_branch_short just by looking at the diff. Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness. To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance. The reason for the patch isn't mentioned explicitly. Perhaps it should be. I felt that using something that's already there instead of recreating it is motivation alone. Why? Because it simplifies the code. That's the real answer. I don't disagree. Yet your commit message doesn't explain that anywhere. -- Felipe Contreras -- 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
