Thread: Re: Fixes for 8.1 run of pgindent
> I think we should rerun pgindent on 8.1.X and HEAD to correct > the reported problems. I am betting 90% of our patches > either come from CVS head or 8.1.X branches greater than 8.1.0. Do we really want to run cosmetic cleanups on a stable branch? I'm sure it *should* be safe, it just seems "against the rules" :-) //Magnus
Magnus Hagander wrote: > > I think we should rerun pgindent on 8.1.X and HEAD to correct > > the reported problems. I am betting 90% of our patches > > either come from CVS head or 8.1.X branches greater than 8.1.0. > > Do we really want to run cosmetic cleanups on a stable branch? > > I'm sure it *should* be safe, it just seems "against the rules" :-) Agreed, it is not a great idea, but if we don't, then 8.1.X and CVS HEAD will not match indenting, and patches generated by 8.1.X users will not apply cleanly to CVS HEAD. And if we don't run it at all, we then will have CVS HEAD with columns > 80 and incorrect typedef indentations. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Magnus Hagander wrote:
>> Do we really want to run cosmetic cleanups on a stable branch?
> Agreed, it is not a great idea, but if we don't, then 8.1.X and CVS HEAD
> will not match indenting, and patches generated by 8.1.X users will not
> apply cleanly to CVS HEAD. And if we don't run it at all, we then will
> have CVS HEAD with columns > 80 and incorrect typedef indentations.
I agree with Bruce here: better to keep 8.1 and HEAD matching as best we
can. I've already had problems with back-patching because the comment
indentation in 8.0 and 8.1 is so completely different --- manually
redoing a patch because patch can't figure it out is no fun and a likely
source of errors. Having to do it an extra time for 8.1 vs HEAD would
increase the pain and risk that much more.
One of the reasons I wanted Bruce to post the proposed diff was so that
we could eyeball-verify that it's only hitting comments. I think it's
worth a little more trouble to check the results given that we plan to
run it against 8.1.
regards, tom lane
> > Magnus Hagander wrote: > >> Do we really want to run cosmetic cleanups on a stable branch? > > > Agreed, it is not a great idea, but if we don't, then 8.1.X and CVS > > HEAD will not match indenting, and patches generated by 8.1.X users > > will not apply cleanly to CVS HEAD. And if we don't run it > at all, we > > then will have CVS HEAD with columns > 80 and incorrect > typedef indentations. > > I agree with Bruce here: better to keep 8.1 and HEAD matching > as best we can. I've already had problems with back-patching > because the comment indentation in 8.0 and 8.1 is so > completely different --- manually redoing a patch because > patch can't figure it out is no fun and a likely source of > errors. Having to do it an extra time for 8.1 vs HEAD would > increase the pain and risk that much more. I didn't consider the patch-conflict issue. With that in mind, yeah, it seems reasonable to do it. > One of the reasons I wanted Bruce to post the proposed diff > was so that we could eyeball-verify that it's only hitting > comments. I think it's worth a little more trouble to check > the results given that we plan to run it against 8.1. Sounds like a good idae. //Magnus
Tom Lane wrote: > One of the reasons I wanted Bruce to post the proposed diff was so that > we could eyeball-verify that it's only hitting comments. I think it's > worth a little more trouble to check the results given that we plan to > run it against 8.1. It would be nice if the developers could run pgindent easily on their local trees to minimize conflicts. I'm playing lazy here because I don't want to go get the NetBSD indent. I wonder why we don't include the whole source instead of just a patch? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Magnus Hagander wrote: > >> Do we really want to run cosmetic cleanups on a stable branch? > > > Agreed, it is not a great idea, but if we don't, then 8.1.X and CVS HEAD > > will not match indenting, and patches generated by 8.1.X users will not > > apply cleanly to CVS HEAD. And if we don't run it at all, we then will > > have CVS HEAD with columns > 80 and incorrect typedef indentations. > > I agree with Bruce here: better to keep 8.1 and HEAD matching as best we > can. I've already had problems with back-patching because the comment > indentation in 8.0 and 8.1 is so completely different --- manually > redoing a patch because patch can't figure it out is no fun and a likely > source of errors. Having to do it an extra time for 8.1 vs HEAD would > increase the pain and risk that much more. > > One of the reasons I wanted Bruce to post the proposed diff was so that > we could eyeball-verify that it's only hitting comments. I think it's > worth a little more trouble to check the results given that we plan to > run it against 8.1. I have updated the ftp://candle.pha.pa.us/pub/postgresql/mypatches/indent.diff file so that it doesn't have those conflicting typedef/variable entries. The new diff has fewer uglifications in the date/time routines, and keywords.c is fine now. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Alvaro Herrera wrote: > Tom Lane wrote: > > > One of the reasons I wanted Bruce to post the proposed diff was so that > > we could eyeball-verify that it's only hitting comments. I think it's > > worth a little more trouble to check the results given that we plan to > > run it against 8.1. > > It would be nice if the developers could run pgindent easily on their > local trees to minimize conflicts. I'm playing lazy here because I > don't want to go get the NetBSD indent. I wonder why we don't include > the whole source instead of just a patch? The entire NetBSD indent, already patched, is on our FTP server. Isn't that good enough? I can easily add the entire thing to CVS if people want that. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Alvaro Herrera wrote:
>> It would be nice if the developers could run pgindent easily on their
>> local trees to minimize conflicts.
> The entire NetBSD indent, already patched, is on our FTP server. Isn't
> that good enough?
News to me, and I guess to Alvaro too. Is this mentioned in the
Developer FAQ?
> I can easily add the entire thing to CVS if people want that.
How big is it? If it's not large, and we're not depending on any
upstream maintenance to happen anymore, we could just dump it into
our own CVS.
(Of course this just begs the question of whether we could convert
over to GNU indent. But I suppose that isn't a realistic option
for the current go-round.)
regards, tom lane
Tom Lane wrote: > (Of course this just begs the question of whether we could convert > over to GNU indent. But I suppose that isn't a realistic option > for the current go-round.) Yeah, I was wondering the same thing yesterday. The README in the pgindent directory mentions a GNU indent version not being useful, but it's the version that was current in 2000. In a GNU mirror I see that the tarball increased about 6 times in size in the meantime. Maybe it has gotten somewhat better. -- Alvaro Herrera http://www.amazon.com/gp/registry/DXLWNGRJD34J Syntax error: function hell() needs an argument. Please choose what hell you want to involve.
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Alvaro Herrera wrote: > >> It would be nice if the developers could run pgindent easily on their > >> local trees to minimize conflicts. > > > The entire NetBSD indent, already patched, is on our FTP server. Isn't > > that good enough? > > News to me, and I guess to Alvaro too. Is this mentioned in the > Developer FAQ? It is mentioned in the pgindent README, the idea being if you are going to use pgindent, you would look at the README. > > I can easily add the entire thing to CVS if people want that. > > How big is it? If it's not large, and we're not depending on any > upstream maintenance to happen anymore, we could just dump it into > our own CVS. It is 160k. I don't think it is being actively developed anymore. > (Of course this just begs the question of whether we could convert > over to GNU indent. But I suppose that isn't a realistic option > for the current go-round.) Right. Needs research. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Alvaro Herrera wrote: > Tom Lane wrote: > > > (Of course this just begs the question of whether we could convert > > over to GNU indent. But I suppose that isn't a realistic option > > for the current go-round.) > > Yeah, I was wondering the same thing yesterday. The README in the > pgindent directory mentions a GNU indent version not being useful, but > it's the version that was current in 2000. In a GNU mirror I see that > the tarball increased about 6 times in size in the meantime. Maybe it > has gotten somewhat better. True, but the last update was 2002 so I question whether it is being actively developed now either. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073