Re: [HACKERS] pgindent-polluted commits

2016-01-18 Thread Noah Misch 
On Sat, Jan 16, 2016 at 09:57:45AM +, Simon Riggs wrote:
> On 16 January 2016 at 02:10, Noah Misch  wrote:
> > On Wed, Jan 13, 2016 at 12:13:11PM -0500, Tom Lane wrote:
> > > Basically this is trading off convenience of the committer (all of the
> > > alternatives Noah mentions are somewhat annoying) versus the convenience
> > > of post-commit reviewers.  I'm not sure that his recommendation is the
> > > best trade-off, nor that the situation is precisely comparable to
> > > pre-commit review.  There definitely will be pre-commit review, there
> > > may or may not be any post-commit review.
> >
> > That's a good summary.

> My objective in committing patches to PostgreSQL is to develop the Open
> Source version of PostgreSQL as a standalone product and I encourage others
> to do the same.
> 
> PostgreSQL is open source and therefore usable for various additional
> purposes, one of which is modified versions of PostgreSQL.
> 
> I will not go out of my way to cause problems for the secondary users of
> the code. I will try to implement one of the suggestions for whitespace
> handling, though may make mistakes in that, nobody being perfect.

Thanks.  Clean commits help so many audiences, including immediate post-commit
reviewers, intensive beta testers, fork maintainers, and hackers performing
root cause analysis on the bugs to be discovered in future years.  For what
it's worth, most committers already have been using some mix of strategy 2
(leave pgindent entirely to Bruce) and strategy 1 (neither add nor remove work
for the next whole-tree pgindent to do).  If you're already in that majority,
I advise no change.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-16 Thread Simon Riggs 
On 16 January 2016 at 02:10, Noah Misch  wrote:

> On Wed, Jan 13, 2016 at 12:13:11PM -0500, Tom Lane wrote:
> > Simon Riggs  writes:
> > > On 13 January 2016 at 14:48, Noah Misch  wrote:
> > >> I've noticed commits, from a few of you, carrying pgindent changes to
> lines
> > >> the patch would not otherwise change.
> >
> > > Could we review again why this matters?
> >
> > Basically this is trading off convenience of the committer (all of the
> > alternatives Noah mentions are somewhat annoying) versus the convenience
> > of post-commit reviewers.  I'm not sure that his recommendation is the
> > best trade-off, nor that the situation is precisely comparable to
> > pre-commit review.  There definitely will be pre-commit review, there
> > may or may not be any post-commit review.
>
> That's a good summary.
>
> > I'm willing to go with the "separate commit to reindent individual files"
> > approach if there's a consensus that that makes for a cleaner git
> history.
> > But I'm not 100% convinced it matters.
>
> Thanks.
>

My objective in committing patches to PostgreSQL is to develop the Open
Source version of PostgreSQL as a standalone product and I encourage others
to do the same.

PostgreSQL is open source and therefore usable for various additional
purposes, one of which is modified versions of PostgreSQL.

I will not go out of my way to cause problems for the secondary users of
the code. I will try to implement one of the suggestions for whitespace
handling, though may make mistakes in that, nobody being perfect.

The secondary purposes of the code can only occur if the core code lives
and breathes, so I expect such users to make positive contributions to core
directly and not to block or slow down inclusion of features by others.
Quid pro quo.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] pgindent-polluted commits

2016-01-15 Thread Noah Misch 
On Wed, Jan 13, 2016 at 12:13:11PM -0500, Tom Lane wrote:
> Simon Riggs  writes:
> > On 13 January 2016 at 14:48, Noah Misch  wrote:
> >> I've noticed commits, from a few of you, carrying pgindent changes to lines
> >> the patch would not otherwise change.
> 
> > Could we review again why this matters?
> 
> Basically this is trading off convenience of the committer (all of the
> alternatives Noah mentions are somewhat annoying) versus the convenience
> of post-commit reviewers.  I'm not sure that his recommendation is the
> best trade-off, nor that the situation is precisely comparable to
> pre-commit review.  There definitely will be pre-commit review, there
> may or may not be any post-commit review.

That's a good summary.

> I'm willing to go with the "separate commit to reindent individual files"
> approach if there's a consensus that that makes for a cleaner git history.
> But I'm not 100% convinced it matters.

Thanks.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-14 Thread Andrew Dunstan 



On 01/13/2016 12:13 PM, Tom Lane wrote:

Simon Riggs  writes:

On 13 January 2016 at 14:48, Noah Misch  wrote:

I've noticed commits, from a few of you, carrying pgindent changes to lines
the patch would not otherwise change.

Could we review again why this matters?

Basically this is trading off convenience of the committer (all of the
alternatives Noah mentions are somewhat annoying) versus the convenience
of post-commit reviewers.  I'm not sure that his recommendation is the
best trade-off, nor that the situation is precisely comparable to
pre-commit review.  There definitely will be pre-commit review, there
may or may not be any post-commit review.

I'm willing to go with the "separate commit to reindent individual files"
approach if there's a consensus that that makes for a cleaner git history.
But I'm not 100% convinced it matters.






I do think it makes life easier when going through the git history if 
semantic changes are separated from formatting changes.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-14 Thread Robert Haas 
On Thu, Jan 14, 2016 at 11:25 AM, Andrew Dunstan  wrote:
> I do think it makes life easier when going through the git history if
> semantic changes are separated from formatting changes.

I agree.  And I agree with Mark Dilger's point, too.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-13 Thread Mark Dilger 

> On Jan 13, 2016, at 9:13 AM, Tom Lane  wrote:
> 
> Simon Riggs  writes:
>> On 13 January 2016 at 14:48, Noah Misch  wrote:
>>> I've noticed commits, from a few of you, carrying pgindent changes to lines
>>> the patch would not otherwise change.
> 
>> Could we review again why this matters?
> 
> Basically this is trading off convenience of the committer (all of the
> alternatives Noah mentions are somewhat annoying) versus the convenience
> of post-commit reviewers.  I'm not sure that his recommendation is the
> best trade-off, nor that the situation is precisely comparable to
> pre-commit review.  There definitely will be pre-commit review, there
> may or may not be any post-commit review.
> 
> I'm willing to go with the "separate commit to reindent individual files"
> approach if there's a consensus that that makes for a cleaner git history.
> But I'm not 100% convinced it matters.

As somebody who maintains a fork of the code base, I can say it is easier
to deal with merge conflicts when work is broken out into smaller commits.
Separating whitespace and formatting changes into their own commits
would make my life a little easier.

OTOH, I don't know if the core developer community cares about the ease
of maintaining code forks.


mark



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] pgindent-polluted commits

2016-01-13 Thread Noah Misch 
I've noticed commits, from a few of you, carrying pgindent changes to lines
the patch would not otherwise change.  (That is to say, the next pgindent run
would have made the same changes anyway.)  From
https://wiki.postgresql.org/wiki/Submitting_a_Patch#Reasons_your_patch_might_be_returned:

  The fastest way to get your patch rejected is to make unrelated changes.
  Reformatting lines that haven't changed, changing unrelated comments you
  felt were poorly worded, touching code not necessary to your change, etc.

Commits should follow the same high standards we ask of submissions.  Several
pgindent strategies do conform:

1) Run pgindent, then manually reduce its changes to the subset that your
   patch caused.
2) Don't run pgindent yourself; commit code that pgindent may later change.
3) Push a commit containing nothing but a pgindent run of the files you care
   about, then push a second commit for your feature/bugfix.

Please use of one of those next time you'd run pgindent.

Thanks,
nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-13 Thread Tom Lane 
Simon Riggs  writes:
> On 13 January 2016 at 14:48, Noah Misch  wrote:
>> I've noticed commits, from a few of you, carrying pgindent changes to lines
>> the patch would not otherwise change.

> Could we review again why this matters?

Basically this is trading off convenience of the committer (all of the
alternatives Noah mentions are somewhat annoying) versus the convenience
of post-commit reviewers.  I'm not sure that his recommendation is the
best trade-off, nor that the situation is precisely comparable to
pre-commit review.  There definitely will be pre-commit review, there
may or may not be any post-commit review.

I'm willing to go with the "separate commit to reindent individual files"
approach if there's a consensus that that makes for a cleaner git history.
But I'm not 100% convinced it matters.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgindent-polluted commits

2016-01-13 Thread Simon Riggs 
On 13 January 2016 at 14:48, Noah Misch  wrote:


> I've noticed commits, from a few of you, carrying pgindent changes to lines
> the patch would not otherwise change.


Could we review again why this matters?

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services