Обсуждение: Re: [COMMITTERS] pgsql: Remove outdated comments from the regression test files.

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

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. +

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

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

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

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

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/

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

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/

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

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