Обсуждение: Time to put context diffs in the grave
We haven't insisted on context diffs in years now, and one of my interlocutors has just turned handsprings trying to follow the advice at <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first patch. Unless someone objects really violently I'm going to rip all that stuff out and let sanity prevail. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, On 2018-05-21 21:51:11 -0400, Andrew Dunstan wrote: > We haven't insisted on context diffs in years now, and one of my > interlocutors has just turned handsprings trying to follow the advice at > <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first > patch. > > > Unless someone objects really violently I'm going to rip all that stuff out > and let sanity prevail. Yes. Please. Greetings, Andres Freund
On Mon, May 21, 2018 at 09:51:11PM -0400, Andrew Dunstan wrote: > > We haven't insisted on context diffs in years now, and one of my > interlocutors has just turned handsprings trying to follow the advice at > <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first > patch. > > Unless someone objects really violently I'm going to rip all that stuff out > and let sanity prevail. +10 for sanity! Best, David. -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Mon, May 21, 2018 at 09:51:11PM -0400, Andrew Dunstan wrote: > Unless someone objects really violently I'm going to rip all that stuff out > and let sanity prevail. Please! -- Michael
Вложения
At Mon, 21 May 2018 21:51:11 -0400, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote in <247f4d65-31f3-d426-2b24-fc434fa67286@2ndQuadrant.com> > > We haven't insisted on context diffs in years now, and one of my > interlocutors has just turned handsprings trying to follow the advice > at <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his > first patch. > > > Unless someone objects really violently I'm going to rip all that > stuff out and let sanity prevail. +1. filterdiff has a bug that we can lose a part of hunks with it, and I actually have stepped on it. I saw Álvaro made a complaint about the bug but it doesn't seem to have been fixed. It is the most major reason I'm posting patches in unified format hesitently (really I am!). The second reason is git format-patch doesn't give me diffs in context format. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On Tue, May 22, 2018 at 06:59:56PM +0900, Kyotaro HORIGUCHI wrote: > filterdiff has a bug that we can lose a part of hunks with it, > and I actually have stepped on it. I saw Álvaro made a complaint > about the bug but it doesn't seem to have been fixed. It is the > most major reason I'm posting patches in unified format > hesitently (really I am!). The second reason is git format-patch > doesn't give me diffs in context format. Yeah, I have been bitten by that one too, as well as others: https://www.postgresql.org/message-id/CAJghg4J6Anwob_c-g8B6CX6bzh81L9qNnB0f44r-x59fhMugEg@mail.gmail.com https://www.postgresql.org/message-id/20170913.174239.25978735.horiguchi.kyotaro@lab.ntt.co.jp -- Michael
Вложения
On Mon, May 21, 2018 at 10:52 PM, Andres Freund <andres@anarazel.de> wrote: > Hi, > > On 2018-05-21 21:51:11 -0400, Andrew Dunstan wrote: >> We haven't insisted on context diffs in years now, and one of my >> interlocutors has just turned handsprings trying to follow the advice at >> <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first >> patch. >> >> >> Unless someone objects really violently I'm going to rip all that stuff out >> and let sanity prevail. > > Yes. Please. > OK, that's done. Now I think we can get rid of git-external-diff. While we're about it, does anyone use make_diff any more? It seems rather ancient and crufty. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> OK, that's done. Now I think we can get rid of git-external-diff.
I for one rely on that. I won't tell anyone else what kind of diff
they have to read, but if you try to tell me what kind of diff I have
to read, I'm going to complain.
> While we're about it, does anyone use make_diff any more? It seems
> rather ancient and crufty.
Not me, but judging from the README, possibly Bruce still uses it.
In any case, is it hurting anything?
regards, tom lane
On Wed, May 23, 2018 at 5:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> OK, that's done. Now I think we can get rid of git-external-diff. > > I for one rely on that. I won't tell anyone else what kind of diff > they have to read, but if you try to tell me what kind of diff I have > to read, I'm going to complain. > OK, then given I have just removed all reference to it in the "Working with Git" wiki page, maybe it needs some comments on how to use it :-) >> While we're about it, does anyone use make_diff any more? It seems >> rather ancient and crufty. > > Not me, but judging from the README, possibly Bruce still uses it. > In any case, is it hurting anything? > No, just being anally retentive. But then we saw recently how that can be a good thing :-) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, May 23, 2018 at 05:01:58PM -0400, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > > OK, that's done. Now I think we can get rid of git-external-diff. > > I for one rely on that. I won't tell anyone else what kind of diff > they have to read, but if you try to tell me what kind of diff I have > to read, I'm going to complain. > > > While we're about it, does anyone use make_diff any more? It seems > > rather ancient and crufty. > > Not me, but judging from the README, possibly Bruce still uses it. I have local copies, so they can be removed. I added them years ago in case they helped anyone else. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Hi, On 2018-05-21 21:51:11 -0400, Andrew Dunstan wrote: > > We haven't insisted on context diffs in years now, and one of my > interlocutors has just turned handsprings trying to follow the advice at > <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first > patch. > > > Unless someone objects really violently I'm going to rip all that stuff out > and let sanity prevail. Could we please also change pg_regress' diff invocations? I find it really painful to see differences in buildfarm output due to the -C3. Locally I've long exported PG_REGRESS_DIFF_OPTS, but that doesn't help on the BF. Greetings, Andres Freund
On 06/19/2018 04:54 PM, Andres Freund wrote: > Hi, > > On 2018-05-21 21:51:11 -0400, Andrew Dunstan wrote: >> We haven't insisted on context diffs in years now, and one of my >> interlocutors has just turned handsprings trying to follow the advice at >> <https://wiki.postgresql.org/wiki/Working_with_Git> to produce his first >> patch. >> >> >> Unless someone objects really violently I'm going to rip all that stuff out >> and let sanity prevail. > Could we please also change pg_regress' diff invocations? I find it > really painful to see differences in buildfarm output due to the > -C3. Locally I've long exported PG_REGRESS_DIFF_OPTS, but that doesn't > help on the BF. > It should do if you put it on the build_env stanza of your config file. I just looked at your animal skink and didn't see any trace of it. What PG_REGRESS_DIFF_OPTS do you use? But of course this won;t help you with other peoples' animals :-) One idea I have been playing with is breaking up the reports so that instead of a large text blob we have a series of files. Allowing different formats on the web interface for diff files wouldn't be terribly difficult once we had that - we'd just need to invoke filterdiff or some such. Of course, we couldn't supply more context that was there in the first place ;-) It would be a substantial change, though, so I'm still working on it conceptually. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-06-19 17:18:32 -0400, Andrew Dunstan wrote: > On 06/19/2018 04:54 PM, Andres Freund wrote: > > Could we please also change pg_regress' diff invocations? I find it > > really painful to see differences in buildfarm output due to the > > -C3. Locally I've long exported PG_REGRESS_DIFF_OPTS, but that doesn't > > help on the BF. > It should do if you put it on the build_env stanza of your config file. I > just looked at your animal skink and didn't see any trace of it. What > PG_REGRESS_DIFF_OPTS do you use? > > But of course this won;t help you with other peoples' animals :-) Oh, I was talking about the wider buildfarm, rather than just mine. > One idea I have been playing with is breaking up the reports so that instead > of a large text blob we have a series of files. Allowing different formats > on the web interface for diff files wouldn't be terribly difficult once we > had that - we'd just need to invoke filterdiff or some such. Of course, we > couldn't supply more context that was there in the first place ;-) I mean that would be good - I've way too often spent considerable time looking for errors - but I don't see why that would be related to this change. Given that we've "officially" stopped relying on context diffs, I don't see why we'd not also retire them in pg_regress. Greetings, Andres Freund
On 06/19/2018 05:21 PM, Andres Freund wrote: > Given that we've "officially" stopped relying on context diffs, > I don't see why we'd not also retire them in pg_regress. > Well I suspect at least one person would be made unhappy ;-) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 06/19/2018 05:21 PM, Andres Freund wrote:
>> Given that we've "officially" stopped relying on context diffs,
>> I don't see why we'd not also retire them in pg_regress.
> Well I suspect at least one person would be made unhappy ;-)
Not requiring them is considerably different from actually trying
to stamp them out. To my mind, where we're trying to go here is
not having one set of people impose their diff format preferences
on other people.
There's already a (perhaps underdocumented) way to make regression
diffs look the way you want in local runs. Andrew's idea of getting
the buildfarm server to display diffs either way would go a long
way towards having that same flexibility for buildfarm results, though
it does seem like rather a lot of work for a small point :-(
regards, tom lane
On 2018-06-19 17:41:00 -0400, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > > On 06/19/2018 05:21 PM, Andres Freund wrote: > >> Given that we've "officially" stopped relying on context diffs, > >> I don't see why we'd not also retire them in pg_regress. > > > Well I suspect at least one person would be made unhappy ;-) > > Not requiring them is considerably different from actually trying > to stamp them out. Sure - but I'm not trying to get rid of PG_REGRESS_DIFF_OPTS, just talking about changing the defaults. > There's already a (perhaps underdocumented) way to make regression > diffs look the way you want in local runs. Andrew's idea of getting > the buildfarm server to display diffs either way would go a long > way towards having that same flexibility for buildfarm results, though > it does seem like rather a lot of work for a small point :-( Yea, if this were all already available, I'd be happy with it. But... Greetings, Andres Freund
On Tue, Jun 19, 2018 at 2:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Not requiring them is considerably different from actually trying > to stamp them out. To my mind, where we're trying to go here is > not having one set of people impose their diff format preferences > on other people. Am I the only one that doesn't use context diffs and also doesn't hate them with a passion? I don't get it. > There's already a (perhaps underdocumented) way to make regression > diffs look the way you want in local runs. Actually, it is documented. -- Peter Geoghegan
On 06/19/2018 05:45 PM, Peter Geoghegan wrote: > On Tue, Jun 19, 2018 at 2:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Not requiring them is considerably different from actually trying >> to stamp them out. To my mind, where we're trying to go here is >> not having one set of people impose their diff format preferences >> on other people. > Am I the only one that doesn't use context diffs and also doesn't hate > them with a passion? No. > I don't get it. But I do get it. I won't even start on the things that bug me :-) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services