Re: [Geany-devel] Commit messages on merges
Am 05.03.2012 00:13, schrieb Matthew Brush: On 12-03-04 01:29 PM, Frank Lanitz wrote: On Sun, 04 Mar 2012 13:01:27 -0800 Matthew Brushmbr...@codebrainz.ca wrote: On 12-03-04 07:07 AM, Colomban Wendling wrote: Le 04/03/2012 09:28, Frank Lanitz a écrit : On Sun, 04 Mar 2012 03:40:29 +0100 Colomban Wendlinglists@herbesfolles.org wrote: IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). I agree. And in second case we have to keep care that merge message is informative enough to don't go into complete tree just to understand what have been done there. Personally I started using the git merge command from command line more often instead of github's web interface as its not satisfying my understanding. Same for me, moreover because I prefer to test the PR locally as a simple branch before doing the merge, so it's not much effort than using the GitHub UI, and it's a lot more powerful. Same here, but I don't think it matters whether using `git merge` or the Github GUI to do it, there's still a need to change the default merge message (apparently). Issue on github is, that you aren't able to change the first line ... ... Which isn't necessarily a bad thing. It keeps them standard and the default first line contains useful information like that it was a merge, the PR #, the person who made the PR and their branch name. In any case, I'm fine with doing it however everyone wants. I use gitk to view the history usually, so it's pretty obvious what all has happened. IMO merge X from Y is perfectly fine. The commit is just that, a merge commit. It isn't necessary to contain more information, since the merged commits are there as well. They are not lost and they still describe what they change. And the merge itself commit doesn't actually contain code changes so it might even be wrong to add a story to it (although describing the rationale of the merge is also good). If merge commits are annoying they can be hidden from the log as I mentioned before. Best regards. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On Sun, 04 Mar 2012 03:40:29 +0100 Colomban Wendling lists@herbesfolles.org wrote: IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). I agree. And in second case we have to keep care that merge message is informative enough to don't go into complete tree just to understand what have been done there. Personally I started using the git merge command from command line more often instead of github's web interface as its not satisfying my understanding. Cheers, Frank -- http://frank.uvena.de/en/ pgpWctMCRO6nv.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On Sat, 03 Mar 2012 18:57:34 -0800 Matthew Brush mbr...@codebrainz.ca wrote: On 12-03-03 06:28 PM, Colomban Wendling wrote: Le 04/03/2012 02:01, Jiří Techet a écrit : On Mon, Feb 27, 2012 at 08:33, Matthew Brushmbr...@codebrainz.ca wrote: On 12-02-26 11:20 PM, Frank Lanitz wrote: Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? [snip] Just to give everyone who hasn't checked the commits an idea of the verbosity that those commit messages has. Is it too verbose? I was trying to add some more detailed info because from my experience even though the patch seems to be clear now, when looking at it one year later I often feel like what does the hell the patch do? and why did I write something like that?. But if it's the preferred way I can move the explanation into the merge comment on github. Nope, it's fine IMO -- and I think Matthew quoted them just to tell Frank that despite the unclear merge message the commits themselves were well explained. Correct, they had some *really* good commit messages. The only problem was with my default merge messages I think. Yes. Message like --- Merged from foo/baa Fixes --- are not very useful. (Just a very pointed example to point it out) Cheers, Frank -- http://frank.uvena.de/en/ pgp8KglRL6feb.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On 12-03-04 07:07 AM, Colomban Wendling wrote: Le 04/03/2012 09:28, Frank Lanitz a écrit : On Sun, 04 Mar 2012 03:40:29 +0100 Colomban Wendlinglists@herbesfolles.org wrote: IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). I agree. And in second case we have to keep care that merge message is informative enough to don't go into complete tree just to understand what have been done there. Personally I started using the git merge command from command line more often instead of github's web interface as its not satisfying my understanding. Same for me, moreover because I prefer to test the PR locally as a simple branch before doing the merge, so it's not much effort than using the GitHub UI, and it's a lot more powerful. Same here, but I don't think it matters whether using `git merge` or the Github GUI to do it, there's still a need to change the default merge message (apparently). Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On Sun, 04 Mar 2012 13:01:27 -0800 Matthew Brush mbr...@codebrainz.ca wrote: On 12-03-04 07:07 AM, Colomban Wendling wrote: Le 04/03/2012 09:28, Frank Lanitz a écrit : On Sun, 04 Mar 2012 03:40:29 +0100 Colomban Wendlinglists@herbesfolles.org wrote: IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). I agree. And in second case we have to keep care that merge message is informative enough to don't go into complete tree just to understand what have been done there. Personally I started using the git merge command from command line more often instead of github's web interface as its not satisfying my understanding. Same for me, moreover because I prefer to test the PR locally as a simple branch before doing the merge, so it's not much effort than using the GitHub UI, and it's a lot more powerful. Same here, but I don't think it matters whether using `git merge` or the Github GUI to do it, there's still a need to change the default merge message (apparently). Issue on github is, that you aren't able to change the first line ... Cheers, Frank -- http://frank.uvena.de/en/ pgplQylGebuYv.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On 12-03-04 01:29 PM, Frank Lanitz wrote: On Sun, 04 Mar 2012 13:01:27 -0800 Matthew Brushmbr...@codebrainz.ca wrote: On 12-03-04 07:07 AM, Colomban Wendling wrote: Le 04/03/2012 09:28, Frank Lanitz a écrit : On Sun, 04 Mar 2012 03:40:29 +0100 Colomban Wendlinglists@herbesfolles.org wrote: IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). I agree. And in second case we have to keep care that merge message is informative enough to don't go into complete tree just to understand what have been done there. Personally I started using the git merge command from command line more often instead of github's web interface as its not satisfying my understanding. Same for me, moreover because I prefer to test the PR locally as a simple branch before doing the merge, so it's not much effort than using the GitHub UI, and it's a lot more powerful. Same here, but I don't think it matters whether using `git merge` or the Github GUI to do it, there's still a need to change the default merge message (apparently). Issue on github is, that you aren't able to change the first line ... ... Which isn't necessarily a bad thing. It keeps them standard and the default first line contains useful information like that it was a merge, the PR #, the person who made the PR and their branch name. In any case, I'm fine with doing it however everyone wants. I use gitk to view the history usually, so it's pretty obvious what all has happened. Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On Mon, Feb 27, 2012 at 08:33, Matthew Brush mbr...@codebrainz.ca wrote: On 12-02-26 11:20 PM, Frank Lanitz wrote: Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? Do not show document change notification dialog when MRU switch is in progress When switching between MRU documents, Geany pops up a dialog about document change even for the intermediate non-final documents. This leads to both reload dialog and document switch dialog displayed at the same time and termination of document switching because the newly displayed dialog takes focus. This patch disables reload checks for the intermediate documents and forces reload check for the final document. Do not ignore system() return value to eliminate compiler warning Drop 'already' from the message in project close confirmation dialog Suppose you have project A open and want to open project B. Then the message saying The 'A' project is already open displays. This is slightly confusing and feels like if you were trying to re-open project A even though you are opening different project. The message without 'already' looks clearer in this context. Modify project dialog signals Rename project-dialog-create signal to project-dialog-open because now the dialog exists all the time and the signal name is misleading. Add project-dialog-close signal to indicate that project dialog has been closed and plugins can remove their tabs when needed. In addition, bump plugin API and ABI version. Just to give everyone who hasn't checked the commits an idea of the verbosity that those commit messages has. Is it too verbose? I was trying to add some more detailed info because from my experience even though the patch seems to be clear now, when looking at it one year later I often feel like what does the hell the patch do? and why did I write something like that?. But if it's the preferred way I can move the explanation into the merge comment on github. By the way, because the patches I submitted weren't related in any way, I think they could have been rebased on top of master instead of doing merge. Cheers, Jiri ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
Le 04/03/2012 02:01, Jiří Techet a écrit : On Mon, Feb 27, 2012 at 08:33, Matthew Brush mbr...@codebrainz.ca wrote: On 12-02-26 11:20 PM, Frank Lanitz wrote: Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? [snip] Just to give everyone who hasn't checked the commits an idea of the verbosity that those commit messages has. Is it too verbose? I was trying to add some more detailed info because from my experience even though the patch seems to be clear now, when looking at it one year later I often feel like what does the hell the patch do? and why did I write something like that?. But if it's the preferred way I can move the explanation into the merge comment on github. Nope, it's fine IMO -- and I think Matthew quoted them just to tell Frank that despite the unclear merge message the commits themselves were well explained. By the way, because the patches I submitted weren't related in any way, I think they could have been rebased on top of master instead of doing merge. Agreed, I prefer not to see merges where there's no relation between several (2+) commits. Cheers, Colomban Cheers, Jiri ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
Le 28/02/2012 06:59, Frank Lanitz a écrit : Am 27.02.2012 08:44, schrieb Lex Trotman: [...] I guess if we can filter out merge commits and only show the real commit information it should be good? (See other message with individual commit messages) Yeah, IMO git gives us lots of un-needed merge messages, not much more we can really say other than merged master into branch, so we will have to filter them for human consumption in newsletters anyway. It's not git. It's most likely githubs's webinterface which is causing the entries I'm not happy about. Using command line git merge -m I just did some cool stuff would be a bit better. Now git log e.g. looks like I agree that the merge commits should be a bit informative on what they do, but the branch name should be a good starting point. In the two examples below the branch name make me clearly think that those branches (actually pull simply requests I guess) simply holds random stuff that doesn't depend on each other. I guess if Jiří made a single PR it's more because he though having one per commit was overkill rather than because these commits had strong relationship. Maybe the project_patches one has some semantic, but I doubt fixes does. commit 3bcd7fc40078efd601f0e9bed8efec971d505db2 Merge: 3d4e8b4 5cc8a96 Author: Matthew Brush mbr...@codebrainz.ca Date: Sun Feb 26 21:04:50 2012 -0800 Merge pull request #19 from techee/fixes Fixes commit 3d4e8b41d419255ee1b0764fb60e45ea588bd800 Merge: d7d5a6d ca9dca9 Author: Matthew Brush mbr...@codebrainz.ca Date: Sun Feb 26 20:50:01 2012 -0800 Merge pull request #25 from techee/project_patches Project patches The alternative is to always re-base before committing merged branches to master, which is probably better since we don't care how the developer got to the end point and all the commits and merges she made on the way, we just care what the commit to master does. No. This will remove most probably of e.g. additional contributors. Rebasing doesn't lose authorship information, it just loses original commits hierarchy. Sometimes this hierarchy is useful (like join_lines patches which makes a whole) and sometimes it is just useless stuff that makes the history less readable (like Jiří's fixes branch where each commit is a whole without relationship with the other). IMO we should not record merges when there is only one single commit or when the commits are unrelated (though the latter should probably be less common) and rather rebase or cherry-pick the commits. However, we must keep the merge when the commits are a whole thing not to lose that information (when several commits are needed to implement a single thing). Regards, Colomban Cheers, Frank ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
Am 27.02.2012 08:44, schrieb Lex Trotman: [...] I guess if we can filter out merge commits and only show the real commit information it should be good? (See other message with individual commit messages) Yeah, IMO git gives us lots of un-needed merge messages, not much more we can really say other than merged master into branch, so we will have to filter them for human consumption in newsletters anyway. git log --no-merges ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
Am 27.02.2012 08:44, schrieb Lex Trotman: [...] I guess if we can filter out merge commits and only show the real commit information it should be good? (See other message with individual commit messages) Yeah, IMO git gives us lots of un-needed merge messages, not much more we can really say other than merged master into branch, so we will have to filter them for human consumption in newsletters anyway. It's not git. It's most likely githubs's webinterface which is causing the entries I'm not happy about. Using command line git merge -m I just did some cool stuff would be a bit better. Now git log e.g. looks like commit 3bcd7fc40078efd601f0e9bed8efec971d505db2 Merge: 3d4e8b4 5cc8a96 Author: Matthew Brush mbr...@codebrainz.ca Date: Sun Feb 26 21:04:50 2012 -0800 Merge pull request #19 from techee/fixes Fixes commit 3d4e8b41d419255ee1b0764fb60e45ea588bd800 Merge: d7d5a6d ca9dca9 Author: Matthew Brush mbr...@codebrainz.ca Date: Sun Feb 26 20:50:01 2012 -0800 Merge pull request #25 from techee/project_patches Project patches The alternative is to always re-base before committing merged branches to master, which is probably better since we don't care how the developer got to the end point and all the commits and merges she made on the way, we just care what the commit to master does. No. This will remove most probably of e.g. additional contributors. Cheers, Frank signature.asc Description: OpenPGP digital signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Commit messages on merges
Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? Cheers, Frank ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On 12-02-26 11:20 PM, Frank Lanitz wrote: Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? Do not show document change notification dialog when MRU switch is in progress When switching between MRU documents, Geany pops up a dialog about document change even for the intermediate non-final documents. This leads to both reload dialog and document switch dialog displayed at the same time and termination of document switching because the newly displayed dialog takes focus. This patch disables reload checks for the intermediate documents and forces reload check for the final document. Do not ignore system() return value to eliminate compiler warning Drop 'already' from the message in project close confirmation dialog Suppose you have project A open and want to open project B. Then the message saying The 'A' project is already open displays. This is slightly confusing and feels like if you were trying to re-open project A even though you are opening different project. The message without 'already' looks clearer in this context. Modify project dialog signals Rename project-dialog-create signal to project-dialog-open because now the dialog exists all the time and the signal name is misleading. Add project-dialog-close signal to indicate that project dialog has been closed and plugins can remove their tabs when needed. In addition, bump plugin API and ABI version. Just to give everyone who hasn't checked the commits an idea of the verbosity that those commit messages has. Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
On 12-02-26 11:20 PM, Frank Lanitz wrote: Hi folks, Just something I thought on last merges based on Jiri's patches. Its hard to understand what this merges do just by reading the commit message. Given, that we want to create the ChangeLog based on git log it will be nearly impossible to create a good ChangeLog/Newsfile if we don't keep care. Not sure how, but can we be more verbose here? I guess if we can filter out merge commits and only show the real commit information it should be good? (See other message with individual commit messages) Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Commit messages on merges
[...] I guess if we can filter out merge commits and only show the real commit information it should be good? (See other message with individual commit messages) Yeah, IMO git gives us lots of un-needed merge messages, not much more we can really say other than merged master into branch, so we will have to filter them for human consumption in newsletters anyway. The alternative is to always re-base before committing merged branches to master, which is probably better since we don't care how the developer got to the end point and all the commits and merges she made on the way, we just care what the commit to master does. Cheers Lex ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
