Thread: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes: > Robert Haas wrote: >> Remove outdated comments from the regression test files. >> >> Since 2004, int2 and int4 operators do detect overflow; this was fixed by >> commit 4171bb869f234281a13bb862d3b1e577bf336242. >> >> Extracted from a larger patch by Andres Freund. > I noticed with this commit that we are referencing pre-git-conversion > git branches, basically adding a dependency on git to our commit > messages. I don't see a problem with this, but did we ever reference > CVS details in CVS commits? I don't remember any. I've usually preferred to use a date, eg, "my patch of 2009-10-07", when referring to previous patches in commit messages. I think people have occasionally mentioned CVS revision IDs, but the folly of that should now be obvious. I agree that reference to a git hash is way way way too fragile and git-centric. regards, tom lane
Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Bruce Momjian
Date:
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Robert Haas wrote: > >> Remove outdated comments from the regression test files. > >> > >> Since 2004, int2 and int4 operators do detect overflow; this was fixed by > >> commit 4171bb869f234281a13bb862d3b1e577bf336242. > >> > >> Extracted from a larger patch by Andres Freund. > > > I noticed with this commit that we are referencing pre-git-conversion > > git branches, basically adding a dependency on git to our commit > > messages. I don't see a problem with this, but did we ever reference > > CVS details in CVS commits? I don't remember any. > > I've usually preferred to use a date, eg, "my patch of 2009-10-07", > when referring to previous patches in commit messages. I think people > have occasionally mentioned CVS revision IDs, but the folly of that > should now be obvious. I agree that reference to a git hash is way > way way too fragile and git-centric. Who's going to be the first to say that being git-centric can't ever be a bad thing? ;-) -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Robert Haas
Date:
On Nov 27, 2010, at 2:49 PM, Bruce Momjian <bruce@momjian.us> wrote: > Who's going to be the first to say that being git-centric can't ever be > a bad thing? ;-) At least for me, referring to it that way makes finding the original patch an order of magnitude faster (git show hash). YMMV. ...Robert
Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes: > On Nov 27, 2010, at 2:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >> Who's going to be the first to say that being git-centric can't ever be >> a bad thing? ;-) > At least for me, referring to it that way makes finding the original patch an order of magnitude faster (git show hash). YMMV. [ shrug... ] You need to take the long view here. We're not working on the assumption that git is the last SCM this project will ever use. Even granting that it is, I don't think git hashes are adequately stable; loading the code into a different repository would likely result in new hashes. And AFAIK there is no mechanism that would fix hash references embedded in commit log messages (or the code). regards, tom lane
Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Robert Haas
Date:
On Sat, Nov 27, 2010 at 3:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Nov 27, 2010, at 2:49 PM, Bruce Momjian <bruce@momjian.us> wrote: >>> Who's going to be the first to say that being git-centric can't ever be >>> a bad thing? ;-) > >> At least for me, referring to it that way makes finding the original patch an order of magnitude faster (git show hash). YMMV. > > [ shrug... ] You need to take the long view here. We're not working on > the assumption that git is the last SCM this project will ever use. > Even granting that it is, I don't think git hashes are adequately stable; > loading the code into a different repository would likely result in new > hashes. And AFAIK there is no mechanism that would fix hash references > embedded in commit log messages (or the code). Well, if we ever did want to rewrite the entire development history (why?) I suppose we could rewrite SHA hashes in the commit messages at the same time. But I think one big advantage of git (or svn, or probably any other post-CVS VCS) is that it has unique IDs for commits. Referring to them as "the commit by so-and-so on such-and-such a date" just on the off chance that we might someday decide to replace those unique IDs with another set of unique IDs doesn't make much sense to me. It makes things more difficult now in the hope that, ten years from now when we switch systems again, it'll be easier to use unstructured text to construct a search command to root through the development history than it will be to map a git commit id onto a commit id in the new system. That's possible, but it's far from obvious. We are database professionals; we ought to believe in the value of unique keys. In fact, I'd go a bit further and say that moving in the direction of MORE unstructured text is the last thing that our commit messages need. Right now, I follow your practice of writing the author of a patch separated from the last paragraph of the commit message by a blank line, additionally noting the reviews and others who have contributed (or who reported the problem). The syntax falls short of machine-parseable, though. Other committers do different things, listing the author at the end of the paragraph of commit text, or preceding it with "Author:", or burying it somewhere in the middle, or even writting "Commit so-and-so's patch to do something-or-other." It is impossible to construct a meaningful history of code contributions without grepping the logs for each person's name individually; that's a lot less helpful than it could be, especially since there's no comprehensive list of name to grep for. Perhaps it's too late to fix up the history (though we could annotate it with git notes if someone were willing to do the legwork), but we could certainly do better going forward if we were so inclined. We ought to be looking for ways to include MORE structured information, not less. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Heikki Linnakangas
Date:
On 28.11.2010 06:59, Robert Haas wrote: > On Sat, Nov 27, 2010 at 3:46 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: >> Robert Haas<robertmhaas@gmail.com> writes: >>> On Nov 27, 2010, at 2:49 PM, Bruce Momjian<bruce@momjian.us> wrote: >>>> Who's going to be the first to say that being git-centric can't ever be >>>> a bad thing? ;-) >> >>> At least for me, referring to it that way makes finding the original patch an order of magnitude faster (git show hash). YMMV. >> >> [ shrug... ] You need to take the long view here. We're not working on >> the assumption that git is the last SCM this project will ever use. >> Even granting that it is, I don't think git hashes are adequately stable; >> loading the code into a different repository would likely result in new >> hashes. And AFAIK there is no mechanism that would fix hash references >> embedded in commit log messages (or the code). > > Well, if we ever did want to rewrite the entire development history > (why?) I suppose we could rewrite SHA hashes in the commit messages at > the same time. But I think one big advantage of git (or svn, or > probably any other post-CVS VCS) is that it has unique IDs for > commits. Referring to them as "the commit by so-and-so on > such-and-such a date" just on the off chance that we might someday > decide to replace those unique IDs with another set of unique IDs > doesn't make much sense to me. It makes things more difficult now in > the hope that, ten years from now when we switch systems again, it'll > be easier to use unstructured text to construct a search command to > root through the development history than it will be to map a git > commit id onto a commit id in the new system. That's possible, but > it's far from obvious. We are database professionals; we ought to > believe in the value of unique keys. Let's do both: "This fixes the bug introduced by the foobar patch from Sep 12th (git commitid a2c23897bc). I like to see the date of the referred patch in the commit message, to get an immediate idea of whether it was a 5-year old change or something from the previous day. But the commitid is also nice so you can immediately copy-paste that without reading through the old commit logs. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Magnus Hagander
Date:
On Mon, Nov 29, 2010 at 13:42, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote: > On 28.11.2010 06:59, Robert Haas wrote: >> >> On Sat, Nov 27, 2010 at 3:46 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: >>> >>> Robert Haas<robertmhaas@gmail.com> writes: >>>> >>>> On Nov 27, 2010, at 2:49 PM, Bruce Momjian<bruce@momjian.us> wrote: >>>>> >>>>> Who's going to be the first to say that being git-centric can't ever be >>>>> a bad thing? ;-) >>> >>>> At least for me, referring to it that way makes finding the original >>>> patch an order of magnitude faster (git show hash). YMMV. >>> >>> [ shrug... ] You need to take the long view here. We're not working on >>> the assumption that git is the last SCM this project will ever use. >>> Even granting that it is, I don't think git hashes are adequately stable; >>> loading the code into a different repository would likely result in new >>> hashes. And AFAIK there is no mechanism that would fix hash references >>> embedded in commit log messages (or the code). >> >> Well, if we ever did want to rewrite the entire development history >> (why?) I suppose we could rewrite SHA hashes in the commit messages at >> the same time. But I think one big advantage of git (or svn, or >> probably any other post-CVS VCS) is that it has unique IDs for >> commits. Referring to them as "the commit by so-and-so on >> such-and-such a date" just on the off chance that we might someday >> decide to replace those unique IDs with another set of unique IDs >> doesn't make much sense to me. It makes things more difficult now in >> the hope that, ten years from now when we switch systems again, it'll >> be easier to use unstructured text to construct a search command to >> root through the development history than it will be to map a git >> commit id onto a commit id in the new system. That's possible, but >> it's far from obvious. We are database professionals; we ought to >> believe in the value of unique keys. > > Let's do both: "This fixes the bug introduced by the foobar patch from Sep > 12th (git commitid a2c23897bc). > > I like to see the date of the referred patch in the commit message, to get > an immediate idea of whether it was a 5-year old change or something from > the previous day. But the commitid is also nice so you can immediately > copy-paste that without reading through the old commit logs. +1. Having the git id is very useful, and putting the date in makes it no *less* informational than what we had before, if/when we move away from git and it's hashes. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Robert Haas
Date:
On Mon, Nov 29, 2010 at 7:45 AM, Magnus Hagander <magnus@hagander.net> wrote: >> Let's do both: "This fixes the bug introduced by the foobar patch from Sep >> 12th (git commitid a2c23897bc). >> >> I like to see the date of the referred patch in the commit message, to get >> an immediate idea of whether it was a 5-year old change or something from >> the previous day. But the commitid is also nice so you can immediately >> copy-paste that without reading through the old commit logs. > > +1. > > Having the git id is very useful, and putting the date in makes it no > *less* informational than what we had before, if/when we move away > from git and it's hashes. That works for me. But should we make a practice of writing the ENTIRE SHA-ID rather than an abbreviated form, so that we could more easily replace 'em later if need be? I think that would be a good idea for other reasons anyway - it's always possible - though admittedly unlikely - that a later commit could introduce a conflict with the first 10 characters, but a conflict with the whole string is pretty much discountable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Magnus Hagander
Date:
On Mon, Nov 29, 2010 at 16:26, Robert Haas <robertmhaas@gmail.com> wrote: > On Mon, Nov 29, 2010 at 7:45 AM, Magnus Hagander <magnus@hagander.net> wrote: >>> Let's do both: "This fixes the bug introduced by the foobar patch from Sep >>> 12th (git commitid a2c23897bc). >>> >>> I like to see the date of the referred patch in the commit message, to get >>> an immediate idea of whether it was a 5-year old change or something from >>> the previous day. But the commitid is also nice so you can immediately >>> copy-paste that without reading through the old commit logs. >> >> +1. >> >> Having the git id is very useful, and putting the date in makes it no >> *less* informational than what we had before, if/when we move away >> from git and it's hashes. > > That works for me. But should we make a practice of writing the > ENTIRE SHA-ID rather than an abbreviated form, so that we could more > easily replace 'em later if need be? I think that would be a good > idea for other reasons anyway - it's always possible - though > admittedly unlikely - that a later commit could introduce a conflict > with the first 10 characters, but a conflict with the whole string is > pretty much discountable. I think that's a good idea. And I suppose this is just going to be cut&paste in (almost?) every case anyway, so it doesn't really change the effort involved.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
Tom Lane
Date:
Magnus Hagander <magnus@hagander.net> writes: > On Mon, Nov 29, 2010 at 16:26, Robert Haas <robertmhaas@gmail.com> wrote: >> That works for me. �But should we make a practice of writing the >> ENTIRE SHA-ID rather than an abbreviated form, so that we could more >> easily replace 'em later if need be? > I think that's a good idea. Just as a data point, there is already one 7-hex-digit collision in our repository: Branch: master Release: REL6_1 [aaeef4dae] 1997-04-09 08:29:35 +0000 Branch: master Release: REL6_1 [aaeef4d17] 1996-11-10 03:06:38 +0000 I think it's quite foolish to depend on abbreviated hashes to be unique forever. regards, tom lane
Re: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.
From
David Fetter
Date:
On Mon, Nov 29, 2010 at 02:09:29PM -0500, Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: > > On Mon, Nov 29, 2010 at 16:26, Robert Haas <robertmhaas@gmail.com> wrote: > >> That works for me. �But should we make a practice of writing the > >> ENTIRE SHA-ID rather than an abbreviated form, so that we could more > >> easily replace 'em later if need be? > > > I think that's a good idea. > > Just as a data point, there is already one 7-hex-digit collision in our > repository: > > Branch: master Release: REL6_1 [aaeef4dae] 1997-04-09 08:29:35 +0000 > Branch: master Release: REL6_1 [aaeef4d17] 1996-11-10 03:06:38 +0000 > > I think it's quite foolish to depend on abbreviated hashes to be unique > forever. Good point. While a full-hash collision is of course possible, we have much more likely things that can mess us up than that :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate