commit messages
Locked
 
|
Hi all,
Nick's going to think this is directed at him since he just committed, but it's not; I've been observing this for a while now. :) Would committers please do everyone else a favour and include at least *one line* of description about the included change? The common pattern at the moment seems to be to put the issue number in the commit message and the package information in the issue, but this gives no clue what was done to fix the issue. Particularly now that diffs in the emails are turned off, this makes it basically impossible to follow what's going on without loading every committed package and looking at the code.
A simple indication of what class/method you changed/added/deleted would be a start. For example: ----------
Added WAFoo>>bar, which wraps the call to super in an exception handler ---------- Ideally, if it's not obvious, you'd also include a line or two about the reason you made the fix you did.
This should only take you a few seconds and I (along with anyone looking back at the code history in the future) will thank you. Thank you! (see? :) )
Julian
_______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
2011/6/20 Julian Fitzell <[hidden email]>:
> Hi all, > Nick's going to think this is directed at him since he just committed, but > it's not; I've been observing this for a while now. :) > Would committers please do everyone else a favour and include at least *one > line* of description about the included change? The common pattern at the > moment seems to be to put the issue number in the commit message and the > package information in the issue, but this gives no clue what was done to > fix the issue. Particularly now that diffs in the emails are turned off, > this makes it basically impossible to follow what's going on without loading > every committed package and looking at the code. > A simple indication of what class/method you changed/added/deleted would be > a start. For example: > ---------- > Fixed http://code.google.com/p/seaside/issues/detail?id=xxx > Added WAFoo>>bar, which wraps the call to super in an exception handler > ---------- > Ideally, if it's not obvious, you'd also include a line or two about the > reason you made the fix you did. > This should only take you a few seconds and I (along with anyone looking > back at the code history in the future) will thank you. > Thank you! (see? :) ) > Julian So I should copy and paste the description from the linked issue into the commit message? Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
Hi Julian,
Looking at the code I've just committed: Name: Seaside-Session-NickAger.139 Author: NickAger Time: 20 June 2011, 3:51:59 pm UUID: ad79427b-c248-4194-8c18-8207969a74a7 Ancestors: Seaside-Session-pmm.138 fix for http://code.google.com/p/seaside/issues/detail?id=659 when request isPrefetch, WAApplication>>handleFiltered: DNUs with WAResponseGenerator>>doesNotUnderstand: #statusForbidden. Presumably this is OK? However I guess my test check-in wasn't clear as I copy and pasted:
Name: Seaside-Tests-Session-NickAger.45 Author: NickAger Time: 20 June 2011, 3:50:58 pm UUID: 6e63c856-6791-4a8f-b7be-8d864e29f39c Ancestors: Seaside-Tests-Session-pmm.44 fix for http://code.google.com/p/seaside/issues/detail?id=659 when request isPrefetch, WAApplication>>handleFiltered: DNUs with WAResponseGenerator>>doesNotUnderstand: #statusForbidden. and should have read something like: added WAApplicationTest>>testPreFetchIsForbidden to test http://code.google.com/p/seaside/issues/detail?id=659
or have a missed something? _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
On Mon, Jun 20, 2011 at 4:50 PM, Nick Ager <[hidden email]> wrote:
Well, I'm not sure. There was a DNU.... so how did you fix it? Did you add the method somewhere? Change the sender to avoid sending it? Fix a bug that was resulting in the method being called on the wrong receiver?
I'm less concerned about test commits... if what the test tests is interesting, mention it. Probably in this case a pointer to the issue is fine (particularly if the issue at least points to a package commit that has a description of what the fix was). Julian _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
2011/6/20 Julian Fitzell <[hidden email]>:
> > > On Mon, Jun 20, 2011 at 4:50 PM, Nick Ager <[hidden email]> wrote: >> >> Looking at the code I've just committed: >>> >>> Name: Seaside-Session-NickAger.139 >>> >>> Author: NickAger >>> >>> Time: 20 June 2011, 3:51:59 pm >>> >>> UUID: ad79427b-c248-4194-8c18-8207969a74a7 >>> >>> Ancestors: Seaside-Session-pmm.138 >>> >>> fix for http://code.google.com/p/seaside/issues/detail?id=659 >>> >>> when request isPrefetch, WAApplication>>handleFiltered: DNUs with >>> WAResponseGenerator>>doesNotUnderstand: #statusForbidden. >> >> Presumably this is OK? However I guess my test check-in wasn't clear as I >> copy and pasted: > > Well, I'm not sure. There was a DNU.... so how did you fix it? Did you add > the method somewhere? Change the sender to avoid sending it? Fix a bug that > was resulting in the method being called on the wrong receiver? Looking at the code addresses all these issues. In addition Monticello doesn't have something like svn annotate. People will see a commit message once and never again. Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Philippe Marschall
On Mon, Jun 20, 2011 at 4:42 PM, Philippe Marschall <[hidden email]> wrote:
So I should copy and paste the description from the linked issue into Not unless that explains what you changed. I mean, clearly sometimes the description of the issue leaves no ambiguity. e.g.: "Issue xxx: WAPharoPlatform missing implementation of #fooBarBaz" Probably just saying you fixed the issue is ok in that case. But even then, a comment of "implemented it" or "implemented it to call primitive version" or something would be helpful. Usually there are many possible fixes to an issue - what one did you decide on? Just give me enough information to know whether I want to go check the fix further... Julian _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Philippe Marschall
On Mon, Jun 20, 2011 at 5:57 PM, Philippe Marschall <[hidden email]> wrote:
If you can get me commit message diffs back, that's fine. But without those, it's a *lot* more work to go check the code just to find out I didn't care in the first place. All I want is enough context in the commit message to know whether to bother going to look at the code. If the solution seems obvious, I'm probably fine with it. If the solution seems weird, I'd like to go dig further. And I often read the commit messages when looking back through package history in Monticello... (certainly faster than reading the code on anything but a very trivial commit). Julian _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Julian Fitzell-2
2011/6/20 Julian Fitzell <[hidden email]>:
> On Mon, Jun 20, 2011 at 4:42 PM, Philippe Marschall > <[hidden email]> wrote: >> >> So I should copy and paste the description from the linked issue into >> the commit message? > > Not unless that explains what you changed. I mean, clearly sometimes the > description of the issue leaves no ambiguity. e.g.: > > "Issue xxx: WAPharoPlatform missing implementation of #fooBarBaz" > > Probably just saying you fixed the issue is ok in that case. But even then, > a comment of "implemented it" or "implemented it to call primitive version" > or something would be helpful. > > Usually there are many possible fixes to an issue - what one did you decide > on? Just give me enough information to know whether I want to go check the > fix further... I don't get payed enough for such big corporation bullshit. We got here not in spite but because we don't have such policies. Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
FWIW, I agree with Philippe here.
The issue tracking system is where the detailed information should be recorded ... if one is checking in code without recording information in the issue tracking system, then I would agree with Julian's comment, there needs to be additional information, but with a full thread in the issue tracking system there is no need to duplicate the information in the commit message... Dale ----- Original Message ----- | From: "Philippe Marschall" <[hidden email]> | To: "Seaside - developer list" <[hidden email]> | Sent: Monday, June 20, 2011 10:02:53 AM | Subject: Re: [Seaside-dev] commit messages | | 2011/6/20 Julian Fitzell <[hidden email]>: | > On Mon, Jun 20, 2011 at 4:42 PM, Philippe Marschall | > <[hidden email]> wrote: | >> | >> So I should copy and paste the description from the linked issue | >> into | >> the commit message? | > | > Not unless that explains what you changed. I mean, clearly | > sometimes the | > description of the issue leaves no ambiguity. e.g.: | > | > "Issue xxx: WAPharoPlatform missing implementation of #fooBarBaz" | > | > Probably just saying you fixed the issue is ok in that case. But | > even then, | > a comment of "implemented it" or "implemented it to call primitive | > version" | > or something would be helpful. | > | > Usually there are many possible fixes to an issue - what one did | > you decide | > on? Just give me enough information to know whether I want to go | > check the | > fix further... | | I don't get payed enough for such big corporation bullshit. We got | here not in spite but because we don't have such policies. | | Cheers | Philippe | _______________________________________________ | seaside-dev mailing list | [hidden email] | http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev | _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
Well, ok. I disagree (I'll save the details for some other time :) ), but I see the issue change emails too, so as long as you describe what the *fix* was in the issue log, that will serve the same purpose.
And please don't be dramatic, Philippe. I'm asking for a phrase - maybe 20 seconds of your time per commit - to save everybody else following the project from having to load an image, wait an eternity for Monticello to load the package, and study the diff. How many commits have you done in the last month? Are you really telling me 20 seconds for each of those adds up to enough "corporation bullshit" to cause the failure of our project? Sorry, I don't buy it for a second. I was simply asking if people writing code would be willing to do me the favour of investing a few seconds of their time to make life a little easier for people reading it. Julian
On Mon, Jun 20, 2011 at 6:24 PM, Dale Henrichs <[hidden email]> wrote: FWIW, I agree with Philippe here. _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
2011/6/21 Julian Fitzell <[hidden email]>:
> Well, ok. I disagree (I'll save the details for some other time :) ), but I > see the issue change emails too, so as long as you describe what the *fix* > was in the issue log, that will serve the same purpose. > > And please don't be dramatic, Philippe. I'm asking for a phrase - maybe 20 > seconds of your time per commit - to save everybody else following the > project from having to load an image, wait an eternity for Monticello to > load the package, and study the diff. How many commits have you done in the > last month? Are you really telling me 20 seconds for each of those adds up > to enough "corporation bullshit" to cause the failure of our project? Sorry, > I don't buy it for a second. Well how many commits have you done in the last month that I can take as an example? > I was simply asking if people writing code would be willing to do me the > favour of investing a few seconds of their time to make life a little easier > for people reading it. It's totally obvious from the change _and_ the issue what the problem and the fix was. I don't feel the need to repeat myself a third time. For the longest time we wouldn't even have an issue tracker and let alone a change log that would list all the issues that we fixed in a release. I mean seriously now you get all this for free and still manage to complain? Fuck, I don't even complain about the shitty RB anymore. I could also put in static types in the code. I'm sure some people would argue that is very helpful and would save them some time. If you have any question about any code or commit feel free to ask and I'm happy to answer. But even more bureaucratic overhead, no thanks. Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
2011/6/21 Philippe Marschall <[hidden email]>:
> 2011/6/21 Julian Fitzell <[hidden email]>: >> Well, ok. I disagree (I'll save the details for some other time :) ), but I >> see the issue change emails too, so as long as you describe what the *fix* >> was in the issue log, that will serve the same purpose. >> >> And please don't be dramatic, Philippe. I'm asking for a phrase - maybe 20 >> seconds of your time per commit - to save everybody else following the >> project from having to load an image, wait an eternity for Monticello to >> load the package, and study the diff. How many commits have you done in the >> last month? Are you really telling me 20 seconds for each of those adds up >> to enough "corporation bullshit" to cause the failure of our project? Sorry, >> I don't buy it for a second. > > Well how many commits have you done in the last month that I can take > as an example? > >> I was simply asking if people writing code would be willing to do me the >> favour of investing a few seconds of their time to make life a little easier >> for people reading it. > > It's totally obvious from the change _and_ the issue what the problem > and the fix was. I don't feel the need to repeat myself a third time. > For the longest time we wouldn't even have an issue tracker and let > alone a change log that would list all the issues that we fixed in a > release. I mean seriously now you get all this for free and still > manage to complain? Fuck, I don't even complain about the shitty RB > anymore. We didn't even have releases. We started to call it a release when we abounded the code because we moved on. Cheers Philippe _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Julian Fitzell-2
So, I guess this will become a non-issue if/when squeaksource is retired and Seaside moves to squeaksource 3 (http://ss3.gemstone.com/) or Smalltalk hub (http://smalltalkhub.objectfusion.fr) - assuming they provide commit message diffs. Could squeaksource 3 mirror squeaksource and provide the commit message diffs?
_______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Philippe Marschall
Sheesh, Philippe; this seems to have touched a nerve for you.
I'm not going to keep arguing with you. I'll simply say that, while you may feel the fix is obvious, I observe that in many many cases it is not. I simply asked whether commiters would do me (and others) the favour of jotting down their thinking as they commit. If you feel this is too much to ask of your time, then carry on as normal. If other committers are less put out by the request, they'll still have my gratitude and appreciation. Julian On 6/21/11, Philippe Marschall <[hidden email]> wrote: > 2011/6/21 Julian Fitzell <[hidden email]>: >> Well, ok. I disagree (I'll save the details for some other time :) ), but >> I >> see the issue change emails too, so as long as you describe what the *fix* >> was in the issue log, that will serve the same purpose. >> >> And please don't be dramatic, Philippe. I'm asking for a phrase - maybe 20 >> seconds of your time per commit - to save everybody else following the >> project from having to load an image, wait an eternity for Monticello to >> load the package, and study the diff. How many commits have you done in >> the >> last month? Are you really telling me 20 seconds for each of those adds up >> to enough "corporation bullshit" to cause the failure of our project? >> Sorry, >> I don't buy it for a second. > > Well how many commits have you done in the last month that I can take > as an example? > >> I was simply asking if people writing code would be willing to do me the >> favour of investing a few seconds of their time to make life a little >> easier >> for people reading it. > > It's totally obvious from the change _and_ the issue what the problem > and the fix was. I don't feel the need to repeat myself a third time. > For the longest time we wouldn't even have an issue tracker and let > alone a change log that would list all the issues that we fixed in a > release. I mean seriously now you get all this for free and still > manage to complain? Fuck, I don't even complain about the shitty RB > anymore. > > I could also put in static types in the code. I'm sure some people > would argue that is very helpful and would save them some time. > > If you have any question about any code or commit feel free to ask and > I'm happy to answer. But even more bureaucratic overhead, no thanks. > > Cheers > Philippe > _______________________________________________ > seaside-dev mailing list > [hidden email] > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev > seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Nick
Mirroring for SS3 could be accomplished ... If you look at the pharo project on SS3[1], you can see that I've arranged to mirror the SqueakSource pharo project (copied over 9K+ mcz files) by running a simple Gofer script that does an xfer between two repositories (I extended gofer with an 'xfer' command ... but mirror would be a better name:) ..
Dale [1] http://ss3.gemstone.com/ss/pharo.html/Statistics ----- Original Message ----- | From: "Nick Ager" <[hidden email]> | To: "Seaside - developer list" <[hidden email]> | Sent: Tuesday, June 21, 2011 1:25:17 AM | Subject: Re: [Seaside-dev] commit messages | | | | | | | | If you can get me commit message diffs back, that's fine. | | | So, I guess this will become a non-issue if/when squeaksource is | retired and Seaside moves to squeaksource 3 ( | http://ss3.gemstone.com/ ) or Smalltalk hub ( | http://smalltalkhub.objectfusion.fr ) - assuming they provide commit | message diffs. Could squeaksource 3 mirror squeaksource and provide | the commit message diffs? | _______________________________________________ | seaside-dev mailing list | [hidden email] | http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev | _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
Ok, if you create "PortablePetitParser" on gemstone ss, and assign me as a developer (or better, rw rights to everybody), I will commit a first attempt (just a "rename method" refactor on ==> and >=>), and see what happens :)
Esteban El 21/06/2011, a las 1:37p.m., Dale Henrichs escribió: > Mirroring for SS3 could be accomplished ... If you look at the pharo project on SS3[1], you can see that I've arranged to mirror the SqueakSource pharo project (copied over 9K+ mcz files) by running a simple Gofer script that does an xfer between two repositories (I extended gofer with an 'xfer' command ... but mirror would be a better name:) .. > > Dale > > [1] http://ss3.gemstone.com/ss/pharo.html/Statistics > > ----- Original Message ----- > | From: "Nick Ager" <[hidden email]> > | To: "Seaside - developer list" <[hidden email]> > | Sent: Tuesday, June 21, 2011 1:25:17 AM > | Subject: Re: [Seaside-dev] commit messages > | > | > | > | > | > | > | > | If you can get me commit message diffs back, that's fine. > | > | > | So, I guess this will become a non-issue if/when squeaksource is > | retired and Seaside moves to squeaksource 3 ( > | http://ss3.gemstone.com/ ) or Smalltalk hub ( > | http://smalltalkhub.objectfusion.fr ) - assuming they provide commit > | message diffs. Could squeaksource 3 mirror squeaksource and provide > | the commit message diffs? > | _______________________________________________ > | seaside-dev mailing list > | [hidden email] > | http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev > | > _______________________________________________ > seaside-dev mailing list > [hidden email] > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Dale Henrichs
oops, bat post... it was on the other list :)
El 21/06/2011, a las 1:37p.m., Dale Henrichs escribió: > Mirroring for SS3 could be accomplished ... If you look at the pharo project on SS3[1], you can see that I've arranged to mirror the SqueakSource pharo project (copied over 9K+ mcz files) by running a simple Gofer script that does an xfer between two repositories (I extended gofer with an 'xfer' command ... but mirror would be a better name:) .. > > Dale > > [1] http://ss3.gemstone.com/ss/pharo.html/Statistics > > ----- Original Message ----- > | From: "Nick Ager" <[hidden email]> > | To: "Seaside - developer list" <[hidden email]> > | Sent: Tuesday, June 21, 2011 1:25:17 AM > | Subject: Re: [Seaside-dev] commit messages > | > | > | > | > | > | > | > | If you can get me commit message diffs back, that's fine. > | > | > | So, I guess this will become a non-issue if/when squeaksource is > | retired and Seaside moves to squeaksource 3 ( > | http://ss3.gemstone.com/ ) or Smalltalk hub ( > | http://smalltalkhub.objectfusion.fr ) - assuming they provide commit > | message diffs. Could squeaksource 3 mirror squeaksource and provide > | the commit message diffs? > | _______________________________________________ > | seaside-dev mailing list > | [hidden email] > | http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev > | > _______________________________________________ > seaside-dev mailing list > [hidden email] > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
«
Return to Seaside Development
|
1 view|%1 views
| Free forum by Nabble | Edit this page |