Run pgindent now?
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run. It's possible we'd need another one before
9.5 is branched off from HEAD, but a run now ought to take care of 95%
of the cleanup needed. I see a couple of advantages to doing it now:
1. Patches that are now postponed to 9.6 will need to be rebased against
pgindented sources anyway. Might as well give their authors more time
for that rather than less.
2. Code that matches the project layout conventions is easier to read
and review, IMO (not that I'm especially in love with the pgindent
layout, but I'm used to it). Also any issues like commit d678bde65
would be taken care of, which is an objective reason why reviewing is
easier.
The only significant downside I can think of is that, if we determine
that any of the recent feature additions are so sketchy that they need
to be reverted, having to undo just a portion of the pgindent commit
before reverting the feature commit would be a pain. But I don't think
we should optimize on the assumption that that will happen.
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
On Sat, May 16, 2015 at 5:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run. It's possible we'd need another one before
9.5 is branched off from HEAD, but a run now ought to take care of 95%
of the cleanup needed. I see a couple of advantages to doing it now:1. Patches that are now postponed to 9.6 will need to be rebased against
pgindented sources anyway. Might as well give their authors more time
for that rather than less.2. Code that matches the project layout conventions is easier to read
and review, IMO (not that I'm especially in love with the pgindent
layout, but I'm used to it). Also any issues like commit d678bde65
would be taken care of, which is an objective reason why reviewing is
easier.The only significant downside I can think of is that, if we determine
that any of the recent feature additions are so sketchy that they need
to be reverted, having to undo just a portion of the pgindent commit
before reverting the feature commit would be a pain. But I don't think
we should optimize on the assumption that that will happen.
+1, except I suggest we at least delay it until we have wrapped the new
minor releases, to make sure we don't conflict with any backpatching there.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On Sat, May 16, 2015 at 11:58:59AM -0400, Tom Lane wrote:
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run. It's possible we'd need another one before
9.5 is branched off from HEAD, but a run now ought to take care of 95%
of the cleanup needed. I see a couple of advantages to doing it now:
+1 to Magnus's suggestion of doing it after the minor release wrap.
The only significant downside I can think of is that, if we determine
that any of the recent feature additions are so sketchy that they need
to be reverted, having to undo just a portion of the pgindent commit
before reverting the feature commit would be a pain. But I don't think
we should optimize on the assumption that that will happen.
Quite so; we'd have lost our way to be optimizing for that.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Magnus Hagander <magnus@hagander.net> writes:
On Sat, May 16, 2015 at 5:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run.
+1, except I suggest we at least delay it until we have wrapped the new
minor releases, to make sure we don't conflict with any backpatching there.
Sure, a couple of days won't make much difference.
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
On Sat, May 16, 2015 at 01:05:27PM -0400, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Sat, May 16, 2015 at 5:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run.+1, except I suggest we at least delay it until we have wrapped the new
minor releases, to make sure we don't conflict with any backpatching there.Sure, a couple of days won't make much difference.
There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically only
pgindented in head, meaning that any future patches in that area could
not be easily backpatched.
Do we want to do this?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, May 18, 2015 at 12:10 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Sat, May 16, 2015 at 01:05:27PM -0400, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Sat, May 16, 2015 at 5:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
With feature freeze behind us, I'd like to propose that now is a good
time for a pgindent run.+1, except I suggest we at least delay it until we have wrapped the new
minor releases, to make sure we don't conflict with any backpatching there.Sure, a couple of days won't make much difference.
There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically only
pgindented in head, meaning that any future patches in that area could
not be easily backpatched.Do we want to do this?
I am personally not excited about that. I would rather leave the
back-branches alone.
--
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
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, May 18, 2015 at 12:10 PM, Bruce Momjian <bruce@momjian.us> wrote:
There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically only
pgindented in head, meaning that any future patches in that area could
not be easily backpatched.Do we want to do this?
I am personally not excited about that. I would rather leave the
back-branches alone.
It would be awfully nice though if we didn't have to deal with random
cross-branch indenting differences. I've lost, maybe not years off my
life, but certainly weeks of not-very-pleasant make-work because of that.
I'm surprised you've not had the same experience.
If people were good about pgindenting patches meant to be back-patched
*before* they committed, it would not be such an issue, but they're not
very good about that.
Would it alleviate your concern any if we eased into this, like say only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.
(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)
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
On Mon, May 18, 2015 at 06:53:00PM -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, May 18, 2015 at 12:10 PM, Bruce Momjian <bruce@momjian.us> wrote:
There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically only
pgindented in head, meaning that any future patches in that area could
not be easily backpatched.Do we want to do this?
I am personally not excited about that. I would rather leave the
back-branches alone.It would be awfully nice though if we didn't have to deal with random
cross-branch indenting differences. I've lost, maybe not years off my
life, but certainly weeks of not-very-pleasant make-work because of that.
I'm surprised you've not had the same experience.If people were good about pgindenting patches meant to be back-patched
*before* they committed, it would not be such an issue, but they're not
very good about that.
I couldn't figure out why we were getting that code drift, but now that
Tom has identified why it happens, it seems good that we fix it.
Would it alleviate your concern any if we eased into this, like say only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)
Uh, I just happen to commit the typedef list file used for the pgindent
run in src/tools/pgindent/typedefs.list, per branch, so we would just
use the same file. If typedefs were added in a backbranch (unlikely),
we probably wouldn't want to use them anyway.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian <bruce@momjian.us> writes:
On Mon, May 18, 2015 at 06:53:00PM -0400, Tom Lane wrote:
(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)
Uh, I just happen to commit the typedef list file used for the pgindent
run in src/tools/pgindent/typedefs.list, per branch, so we would just
use the same file. If typedefs were added in a backbranch (unlikely),
we probably wouldn't want to use them anyway.
Not sure why you think it's unlikely; a back-patched commit could easily
add one. And if it did, we'd want pgindent to treat it the same as in
HEAD, else the whole point of this is gone.
(Come to think of it, that argument means we *do* want to use the same
typedef list in every branch, if we're to do this at all.)
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
On Mon, May 18, 2015 at 07:10:00PM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
On Mon, May 18, 2015 at 06:53:00PM -0400, Tom Lane wrote:
(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)Uh, I just happen to commit the typedef list file used for the pgindent
run in src/tools/pgindent/typedefs.list, per branch, so we would just
use the same file. If typedefs were added in a backbranch (unlikely),
we probably wouldn't want to use them anyway.Not sure why you think it's unlikely; a back-patched commit could easily
add one. And if it did, we'd want pgindent to treat it the same as in
HEAD, else the whole point of this is gone.
Oh, good point.
(Come to think of it, that argument means we *do* want to use the same
typedef list in every branch, if we're to do this at all.)
I am feeling either the head typedefs or the per-branch typedefs are
close enough that either would be fine. If we go with the head
typedefs, there will be some churn in the code layout, though, unrelated
to the patches applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/18/2015 07:04 PM, Bruce Momjian wrote:
On Mon, May 18, 2015 at 06:53:00PM -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, May 18, 2015 at 12:10 PM, Bruce Momjian <bruce@momjian.us> wrote:
There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically only
pgindented in head, meaning that any future patches in that area could
not be easily backpatched.Do we want to do this?
I am personally not excited about that. I would rather leave the
back-branches alone.It would be awfully nice though if we didn't have to deal with random
cross-branch indenting differences. I've lost, maybe not years off my
life, but certainly weeks of not-very-pleasant make-work because of that.
I'm surprised you've not had the same experience.If people were good about pgindenting patches meant to be back-patched
*before* they committed, it would not be such an issue, but they're not
very good about that.I couldn't figure out why we were getting that code drift, but now that
Tom has identified why it happens, it seems good that we fix it.Would it alleviate your concern any if we eased into this, like say only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)Uh, I just happen to commit the typedef list file used for the pgindent
run in src/tools/pgindent/typedefs.list, per branch, so we would just
use the same file. If typedefs were added in a backbranch (unlikely),
we probably wouldn't want to use them anyway.
The buildfarm animals are perfectly capable of finding typedefs for each
branch. They haven't been because the default configuration is only to
collect them for HEAD.
Changing this is easy, especially since I control five of the six
members currently reporting typedefs successfully, and Tom controls the
other one.
I've currently set two of them to do run typedefs for all live branches.
The other thing is that the server script that amalgamates them only
looks at HEAD. That will need to change.
We would probably want an amalgamated list, because there could have
been symbols on old branches that were deleted in later branches. With
luck the presence of false positives wouldn't matter. It usually doesn't
seem to.
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
On 05/18/2015 08:06 PM, Andrew Dunstan wrote:
On 05/18/2015 07:04 PM, Bruce Momjian wrote:
On Mon, May 18, 2015 at 06:53:00PM -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, May 18, 2015 at 12:10 PM, Bruce Momjian <bruce@momjian.us>
wrote:There was talk last time of pgindent-ing head and all back branches,
because a patch applied to head and back branches was historically
only
pgindented in head, meaning that any future patches in that area
could
not be easily backpatched.Do we want to do this?
I am personally not excited about that. I would rather leave the
back-branches alone.It would be awfully nice though if we didn't have to deal with random
cross-branch indenting differences. I've lost, maybe not years off my
life, but certainly weeks of not-very-pleasant make-work because of
that.
I'm surprised you've not had the same experience.If people were good about pgindenting patches meant to be back-patched
*before* they committed, it would not be such an issue, but they're not
very good about that.I couldn't figure out why we were getting that code drift, but now that
Tom has identified why it happens, it seems good that we fix it.Would it alleviate your concern any if we eased into this, like say
only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.(BTW, one practical issue is where would we get typedef lists relevant
to the back branches. I'm not sure if the buildfarm infrastructure is
capable of collecting branch-specific data, or if we'd need to rather
than just using a union of all branches' typedefs.)Uh, I just happen to commit the typedef list file used for the pgindent
run in src/tools/pgindent/typedefs.list, per branch, so we would just
use the same file. If typedefs were added in a backbranch (unlikely),
we probably wouldn't want to use them anyway.The buildfarm animals are perfectly capable of finding typedefs for
each branch. They haven't been because the default configuration is
only to collect them for HEAD.Changing this is easy, especially since I control five of the six
members currently reporting typedefs successfully, and Tom controls
the other one.I've currently set two of them to do run typedefs for all live branches.
The other thing is that the server script that amalgamates them only
looks at HEAD. That will need to change.We would probably want an amalgamated list, because there could have
been symbols on old branches that were deleted in later branches. With
luck the presence of false positives wouldn't matter. It usually
doesn't seem to.
OK, if you look at
<http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list> you will be
able to see the state of things. It's not even remotely pretty, and I am
going to fix that, but it works.
As you will be able to see, a number of buildfarm members are reporting
on typedefs on all the live branches. You can get the list for each
branch by hitting the appropriate link (essentially
'/cgi-bin/typedefs.pl?branch=$branch'). If you ask for 'ALL' as the
branch it gives you the amalgamated list over all branches. If you don't
specify a branch at all, it gives you HEAD (which is buildfarm spelling
for master), since that's what it did previously. I can change the
default to ALL if that's what people want.
Tom, if you want to get dromedary reporting on all branches, just
remove the "branches => [ 'HEAD' ]," from the config.
Enjoy.
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
On Mon, May 18, 2015 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I am personally not excited about that. I would rather leave the
back-branches alone.It would be awfully nice though if we didn't have to deal with random
cross-branch indenting differences. I've lost, maybe not years off my
life, but certainly weeks of not-very-pleasant make-work because of that.
I'm surprised you've not had the same experience.
Well, there are a couple of things that worry me:
- People rely on us to ship, in minor releases, only critical security
and stability fixes. Re-indenting the code is neither, and people may
not appreciate needless whitespace differences being shipped in the
next branch. Anyone who diffs that tarball against the previous one
is going to see a bunch of stuff in there that may make them nervous.
- If pgindent doesn't handle every branch in exactly the same way,
it's possible that this change could exacerbate differences instead of
reducing them. I actually think this is quite a likely outcome.
I personally have not found back-patching to have been significantly
complicated by whitespace differences. There are certainly code
differences that can make it quite miserable in some cases, but I
cannot recall a case where there was an issue of this time due to
erratic indenting in one branch that had meanwhile been fixed in
another branch. I accept that your experience may be different, of
course.
Would it alleviate your concern any if we eased into this, like say only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.
If we do this only beginning with 9.5, and if we can make the output
100% consistent across branches, and if we run it before EVERY minor
release so that people don't see unrelated diffs between consecutive
tarballs, then it would address my concerns.
I wish that pgident could be made more automated, like by having it
fully built into the tree so that you can type 'make indent', or by
having a daemon that would automatically pgindent the main tree
periodically (say, once a month, or when more than X number of
lines/files have changed, whichever comes first). I still find it
quite a hassle to set up and run.
--
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
Andrew Dunstan <andrew@dunslane.net> writes:
Tom, if you want to get dromedary reporting on all branches, just
remove the "branches => [ 'HEAD' ]," from the config.
dromedary is a pretty slow machine, so I'm going to pass on that unless
there's a good reason to think it would find typedefs your machines don't.
I rather doubt that --- our use of platform-dependent typedefs is fairly
small and stable, so it seems like checking HEAD should be sufficient.
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
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, May 18, 2015 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Would it alleviate your concern any if we eased into this, like say only
apply the back-branch pgindent run to 9.5 and later branches? Then at
least I could foresee the end of that particular annoyance.
If we do this only beginning with 9.5, and if we can make the output
100% consistent across branches, and if we run it before EVERY minor
release so that people don't see unrelated diffs between consecutive
tarballs, then it would address my concerns.
To do it before every minor release would require re-indenting HEAD
as well (since the whole point is to keep HEAD and the back branches
consistent). I think we'd get too much push-back from developers
whose pending patches got broken. We can get away with reindenting
HEAD between development cycles, but probably not more often than that.
I'm not particularly concerned by the tarball-diff argument: running
diff with --ignore-spaces should mask most of the changes. Moreover,
assuming the code was properly indented at x.y.0 release time, any
changes applied by pgindent would only be within subsequent back-patches,
which hopefully are a very small part of the code. (Perhaps it would be
useful to do a trial indent on some old branch right now, just to see how
large the diffs are; then we'd have some actual facts in this argument...)
And lastly, committers who are bothered by the prospect of such changes
could take the time to reindent their back-patched changes before
committing in the first place. (FWIW, I usually do, and it's not hard
except in files that have been heavily mangled in HEAD.)
I wish that pgident could be made more automated, like by having it
fully built into the tree so that you can type 'make indent', or by
having a daemon that would automatically pgindent the main tree
periodically (say, once a month, or when more than X number of
lines/files have changed, whichever comes first). I still find it
quite a hassle to set up and run.
It is a pain. I have a shell script that fetches the typedef list
automatically, which helps. The main problem with a "make indent" target
is that only in Bruce's annual runs do we really want to let it loose on
the whole tree. In manual fixups, I only point it at the files I've
edited (and then, often, I have to remove some diffs in unrelated parts
of those files). I wish that could be a bit easier, though I'm not sure
how.
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
On Tue, May 19, 2015 at 10:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
To do it before every minor release would require re-indenting HEAD
as well (since the whole point is to keep HEAD and the back branches
consistent). I think we'd get too much push-back from developers
whose pending patches got broken. We can get away with reindenting
HEAD between development cycles, but probably not more often than that.
I'm not convinced of that. If we did it more often, it might actually
be less disruptive.
I'm not particularly concerned by the tarball-diff argument: running
diff with --ignore-spaces should mask most of the changes. Moreover,
assuming the code was properly indented at x.y.0 release time, any
changes applied by pgindent would only be within subsequent back-patches,
which hopefully are a very small part of the code. (Perhaps it would be
useful to do a trial indent on some old branch right now, just to see how
large the diffs are; then we'd have some actual facts in this argument...)
That parenthetical idea sounds promising.
And lastly, committers who are bothered by the prospect of such changes
could take the time to reindent their back-patched changes before
committing in the first place. (FWIW, I usually do, and it's not hard
except in files that have been heavily mangled in HEAD.)
Meh. With 10+ active committers, that's bound not to always work out.
I wish that pgident could be made more automated, like by having it
fully built into the tree so that you can type 'make indent', or by
having a daemon that would automatically pgindent the main tree
periodically (say, once a month, or when more than X number of
lines/files have changed, whichever comes first). I still find it
quite a hassle to set up and run.It is a pain. I have a shell script that fetches the typedef list
automatically, which helps. The main problem with a "make indent" target
is that only in Bruce's annual runs do we really want to let it loose on
the whole tree. In manual fixups, I only point it at the files I've
edited (and then, often, I have to remove some diffs in unrelated parts
of those files). I wish that could be a bit easier, though I'm not sure
how.
Unless we reindent regularly, the problem with changes in unrelated
parts of the file is not going away. Figuring out which files have
been changed locally could probably be done with some sort of git-fu.
--
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
Robert Haas wrote:
On Tue, May 19, 2015 at 10:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
To do it before every minor release would require re-indenting HEAD
as well (since the whole point is to keep HEAD and the back branches
consistent). I think we'd get too much push-back from developers
whose pending patches got broken. We can get away with reindenting
HEAD between development cycles, but probably not more often than that.I'm not convinced of that. If we did it more often, it might actually
be less disruptive.
I believe it's possible to mechanically rebase a patch over an indent
run of the underlying branch with half a dozen commands or less. +1 for
reindenting all branches before each minor release, FWIW.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Robert Haas wrote:
On Tue, May 19, 2015 at 10:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
To do it before every minor release would require re-indenting HEAD
as well (since the whole point is to keep HEAD and the back branches
consistent). I think we'd get too much push-back from developers
whose pending patches got broken. We can get away with reindenting
HEAD between development cycles, but probably not more often than that.
I'm not convinced of that. If we did it more often, it might actually
be less disruptive.
I believe it's possible to mechanically rebase a patch over an indent
run of the underlying branch with half a dozen commands or less. +1 for
reindenting all branches before each minor release, FWIW.
Yeah? Can you show an example?
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
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I believe it's possible to mechanically rebase a patch over an indent
run of the underlying branch with half a dozen commands or less. +1 for
reindenting all branches before each minor release, FWIW.Yeah? Can you show an example?
So we have this:
---P----I----C'
|
\---C----I'
where P is the parent commit; I is the pgindent commit; C is your
change (applied to the unindented tree). What you need is to obtain C'
which is a copy of C that applies to I. You can do this by creating I'
which is a pgindent run over your patch, then diff that one to I.
I *think* this should work:
git checkout C
pgindent tree
git commit # yields I'
git diff I I' > C'
git checkout I
git apply C'
I spent a few minutes looking for a nontrivial patch to test this on,
couldn't find one; but the key is that you must be able to run pgindent
on your own using the same rules that Bruce's run would.
This shouldn't need human intervention at all, so should even be
possible to write a script for it and use it for
git rebase -i -x this_script origin/master
for when you have a branch with several commits that you want to rebase
over an upstream pgindent.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I believe it's possible to mechanically rebase a patch over an indent
run of the underlying branch with half a dozen commands or less. +1 for
reindenting all branches before each minor release, FWIW.
Yeah? Can you show an example?
I *think* this should work:
git checkout C
pgindent tree
git commit # yields I'
git diff I I' > C'
git checkout I
git apply C'
I spent a few minutes looking for a nontrivial patch to test this on,
couldn't find one; but the key is that you must be able to run pgindent
on your own using the same rules that Bruce's run would.
OK. So agreed, the blocking issue here is whether pgindent is
conveniently available to every patch submitter. Right now, it would
certainly be charitable to describe installing it as a PITA. I think
what we'd need to do is (1) include fully patched sources in our git
tree, and (2) build them by default (but not install them, probably)
so that we can flush out any portability issues.
I think it's too late to consider doing that for 9.5, but maybe we
could do it after the branch.
Another issue is whether there's a copyright problem if we include
modified BSD indent sources in our tree. I wouldn't think so but
we better check exactly how it's licensed.
We'd also want a more mechanical way of obtaining the right typedef list
to use. Although it probably couldn't be totally mechanized, because if
your patch adds new typedefs you'd want to manually add those names to
the list being used. Maybe there should be an optional local typedef
list separate from the automatically generated file. I guess in the
scenario you're describing, the most helpful thing would be if the
pgindent commit put the typedef list it had used into the tree, and then
we just use that (plus manual additions) when generating the I' commit.
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
