Обсуждение: [HACKERS] Patches that don't apply or don't compile: 2017-09-12
Hello, hackers! Thanks to the work of Thomas Munro now we have a CI for the patches on the commitfest [1]. Naturally there is still room for improvement, but in any case it's much, much better than nothing. After a short discussion [2] we agreed (or at least no one objected) to determine the patches that don't apply / don't compile / don't pass regression tests and have "Needs Review" status, change the status of these patches to "Waiting on Author" and write a report (this one) with a CC to the authors. As all we know, we are short on reviewers and this action will save them a lot of time. Here [3] you can find a script I've been using to find such patches. I rechecked the list manually and did my best to exclude the patches that were updated recently or that depend on other patches. However there still a chance that your patch got to the list by a mistake. In this case please let me know. Unless there are any objections I'm going to re-run this script once or twice a week during the commitfest and write reports like this one. Here is the list: === Apply Failed: 25 === Title: 64-bit transaction identifiers Author: Tianzhou Chen <tianzhouchen@gmail.com> (it's a bug - actually Alexander Korotkov) URL: https://commitfest.postgresql.org/14/1178/ Title: Add and report the new "in_hot_standby" GUC pseudo-variable. Author: Elvis Pranskevichus <elprans@gmail.com> URL: https://commitfest.postgresql.org/14/1090/ Title: allow mix of composite and scalar variables in target list Author: Pavel Stehule <pavel.stehule@gmail.com> URL: https://commitfest.postgresql.org/14/1139/ Title: Allow users to specify multiple tables in VACUUM commands Author: "Bossart, Nathan" <bossartn@amazon.com> URL: https://commitfest.postgresql.org/14/1131/ Title: Controlling toast_tuple_target to tune rows >2kB Author: Simon Riggs <simon@2ndquadrant.com> URL: https://commitfest.postgresql.org/14/1284/ Title: Convert join OR clauses into UNION queries Author: Jim Nasby <Jim.Nasby@BlueTreble.com> URL: https://commitfest.postgresql.org/14/1001/ Title: faster partition pruning in planner Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> URL: https://commitfest.postgresql.org/14/1272/ Title: Fix race condition between SELECT and ALTER TABLE NO INHERIT Author: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> URL: https://commitfest.postgresql.org/14/1259/ Title: Gather speed-up Author: Rafia Sabih <rafia.sabih@enterprisedb.com> URL: https://commitfest.postgresql.org/14/1156/ Title: Hash partitioning Authors: Yugo Nagata <nagata@sraoss.co.jp>, "yangjie@highgo.com" <yangjie@highgo.com> URL: https://commitfest.postgresql.org/14/1059/ Title: Improve compactify_tuples and PageRepairFragmentation Author: Sokolov Yura <funny.falcon@postgrespro.ru> URL: https://commitfest.postgresql.org/14/1138/ Title: Improve eval_const_expressions Author: Heikki Linnakangas <hlinnaka@iki.fi> URL: https://commitfest.postgresql.org/14/1136/ Title: Incremental sort Author: Alexander Korotkov <a.korotkov@postgrespro.ru> URL: https://commitfest.postgresql.org/14/1124/ Title: libpq: Support for Apple Secure Transport SSL library Author: Daniel Gustafsson <daniel@yesql.se> URL: https://commitfest.postgresql.org/14/1228/ Title: multivariate MCV lists and histograms Author: Tomas Vondra <tomas.vondra@2ndquadrant.com> URL: https://commitfest.postgresql.org/14/1238/ Title: New function for tsquery creation Author: Victor Drobny <v.drobny@postgrespro.ru> URL: https://commitfest.postgresql.org/14/1202/ Title: pg_basebackup has stricter tablespace rules than the server Author: Pierre Ducroquet <p.psql@pinaraf.info> URL: https://commitfest.postgresql.org/14/1125/ Title: Range Merge Join Author: Jeff Davis <pgsql@j-davis.com> URL: https://commitfest.postgresql.org/14/1106/ Title: Retire polyphase merge Author: Heikki Linnakangas <hlinnaka@iki.fi> URL: https://commitfest.postgresql.org/14/1267/ Title: Subscription code improvements Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> URL: https://commitfest.postgresql.org/14/1248/ Title: Support arrays over domain types Author: Tom Lane <tgl@sss.pgh.pa.us> URL: https://commitfest.postgresql.org/14/1235/ Title: Support target_session_attrs=read-only and eliminate the added round-trip to detect session status. Author: Elvis Pranskevichus <elprans@gmail.com> URL: https://commitfest.postgresql.org/14/1148/ Title: taking stdbool.h into use Author: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> URL: https://commitfest.postgresql.org/14/1242/ Title: UPDATE of partition key Author: Amit Khandekar <amitdkhan.pg@gmail.com> URL: https://commitfest.postgresql.org/14/1023/ Title: Write Amplification Reduction Method (WARM) Author: Pavan Deolasee <pavan.deolasee@gmail.com> URL: https://commitfest.postgresql.org/14/775/ === Build Failed: 7 === Title: Fix the optimization to skip WAL-logging on table created in same transaction Author: Martijn van Oosterhout <kleptog@svana.org> URL: https://commitfest.postgresql.org/14/528/ Title: Foreign Key Arrays Author: Mark Rofail <markm.rofail@gmail.com> URL: https://commitfest.postgresql.org/14/1252/ Title: Generic type subscripting Author: Dmitry Dolgov <9erthalion6@gmail.com> URL: https://commitfest.postgresql.org/14/1062/ Title: new plpgsql extra_checks Author: Pavel Stehule <pavel.stehule@gmail.com> URL: https://commitfest.postgresql.org/14/962/ Title: Replication status in logical replication Author: Masahiko Sawada <sawada.mshk@gmail.com> URL: https://commitfest.postgresql.org/14/1113/ Title: Restricting maximum keep segments by repslots Author: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> URL: https://commitfest.postgresql.org/14/1260/ Title: Temporal query processing with range types Author: Anton Dignös <dignoes@inf.unibz.it> URL: https://commitfest.postgresql.org/14/839/ Needs Review Total: 120 Failed Total: 32 (26.67 %) As always, any feedback is very welcomed! [1]: http://commitfest.cputube.org/ [2]: https://postgr.es/m/CAB7nPqSrHF%2BkNQ6Eq2uy91RcysoCzQr1AjOjkuCn9jvMdJZ6Fw%40mail.gmail.com [3]: https://github.com/afiskon/py-autoreviewer -- Best regards, Aleksander Alekseev
On 09/12/2017 04:14 PM, Aleksander Alekseev wrote: > Title: Foreign Key Arrays > Author: Mark Rofail<markm.rofail@gmail.com> > URL:https://commitfest.postgresql.org/14/1252/ I am currently reviewing this one and it applies, compiles, and passes the test suite. It could be the compilation warnings which makes the system think it failed, but I could not find the log of the failed build. We want to be welcoming to new contributors so until we have a reliable CI server which can provide easy to read build logs I am against changing the status of any patches solely based on the result of the CI server. I think it should be used as a complimentary tool until the community deems it to be good enough. Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Hi Andreas, On 09/12/2017 04:14 PM, Aleksander Alekseev wrote: > > Title: Foreign Key Arrays > > Author: Mark Rofail<markm.rofail@gmail.com> > > URL:https://commitfest.postgresql.org/14/1252/ > > I am currently reviewing this one and it applies, compiles, and passes the > test suite. It could be the compilation warnings which makes the system > think it failed, but I could not find the log of the failed build. Thanks for your feedback. I'm changing the status of this patch back to "Needs Review" and adding it to the exclude list until we will figure out what went wrong. > We want to be welcoming to new contributors so until we have a reliable CI > server which can provide easy to read build logs I am against changing the > status of any patches solely based on the result of the CI server. I think > it should be used as a complimentary tool until the community deems it to be > good enough. Agree, especially regarding build logs. All of this currently is only an experiment. For some reason I got a weird feeling that at this time it will be not quite successful one. If there will be too many false positives I'll just return the patches back to "Needs Review" as it was before. But I strongly believe that after a few iterations we will find a solution that will be good enough and this slight inconveniences will worth it in a long run. -- Best regards, Aleksander Alekseev
On Wed, Sep 13, 2017 at 2:55 AM, Andreas Karlsson <andreas@proxel.se> wrote: > On 09/12/2017 04:14 PM, Aleksander Alekseev wrote: >> >> Title: Foreign Key Arrays >> Author: Mark Rofail<markm.rofail@gmail.com> >> URL:https://commitfest.postgresql.org/14/1252/ > > > I am currently reviewing this one and it applies, compiles, and passes the > test suite. It could be the compilation warnings which makes the system > think it failed, but I could not find the log of the failed build. I guess you didn't run "make check-world", because it crashes in the contrib regression tests: https://travis-ci.org/postgresql-cfbot/postgresql/builds/274732512 Sorry that the build logs are a bit hard to find at the moment. Starting from http://commitfest.cputube.org/ if you click the "build|failing" badge you'll land at https://travis-ci.org/postgresql-cfbot/postgresql/branches and then you have to locate the right branch, in this case commitfest/14/1252, and then click the latest build link which (in this case) looks like "# 4603 failed". Eventually I'll have time to figure out how to make the "build|failing" badge take you there directly... Eventually I'll also teach it how to dump a backtrace out of gdb the tests leave a smouldering core. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 09/12/2017 04:14 PM, Aleksander Alekseev wrote: > Hello, hackers! > > Thanks to the work of Thomas Munro now we have a CI for the patches on > the commitfest [1]. Naturally there is still room for improvement, but > in any case it's much, much better than nothing. > > After a short discussion [2] we agreed (or at least no one objected) > to determine the patches that don't apply / don't compile / don't > pass regression tests and have "Needs Review" status, change the > status of these patches to "Waiting on Author" and write a report > (this one) with a CC to the authors. As all we know, we are short on > reviewers and this action will save them a lot of time. Here [3] you > can find a script I've been using to find such patches. > > I rechecked the list manually and did my best to exclude the patches > that were updated recently or that depend on other patches. However > there still a chance that your patch got to the list by a mistake. > In this case please let me know.> With all due respect, it's hard not to see this as a disruption of the current CF. I agree automating the patch processing is a worthwhile goal, but we're not there yet and it seems somewhat premature. Let me explain why I think so: (1) You just changed the status of 10-15% open patches. I'd expect things like this to be consulted with the CF manager, yet I don't see any comments from Daniel. Considering he's been at the Oslo PUG meetup today, I doubt he was watching hackers very closely. (2) You gave everyone about 4 hours to object, ending 3PM UTC, which excludes about one whole hemisphere where it's either too early or too late for people to respond. I'd say waiting for >24 hours would be more appropriate. (3) The claim that "on one objected" is somewhat misleading, I guess. I myself objected to automating this yesterday, and AFAICS Thomas Munro shares the opinion that we're not ready for automating it. (4) You say you rechecked the list manually - can you elaborate what you checked? Per reports from others, some patches seem to "not apply" merely because "git apply" is quite strict. Have you actually tried applying / compiling the patches yourself? (5) I doubt "does not apply" is actually sufficient to move the patch to "waiting on author". For example my statistics patch was failing to apply merely due to 821fb8cdbf lightly touching the SGML docs, changing "type" to "kind" on a few places. Does that mean the patch can't get any reviews until the author fixes it? Hardly. But after switching it to "waiting on author" that's exactly what's going to happen, as people are mostly ignoring patches in that state. (6) It's generally a good idea to send a message the patch thread whenever the status is changed, otherwise the patch authors may not notice the change for a long time. I don't see any such messages, certainly not in "my" patch thread. I object to changing the patch status merely based on the script output. It's a nice goal, but we need to do the legwork first, otherwise it'll be just annoying and disrupting. I suggest we inspect the reported patches manually, investigate whether the failures are legitimate or not, and eliminate as many false positives as possible. Once we are happy with the accuracy, we can enable it again. kind regards -- Tomas Vondra 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
> On 12 Sep 2017, at 23:54, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote: > > On 09/12/2017 04:14 PM, Aleksander Alekseev wrote: >> Hello, hackers! >> >> Thanks to the work of Thomas Munro now we have a CI for the patches on >> the commitfest [1]. Naturally there is still room for improvement, but >> in any case it's much, much better than nothing. >> >> After a short discussion [2] we agreed (or at least no one objected) >> to determine the patches that don't apply / don't compile / don't >> pass regression tests and have "Needs Review" status, change the >> status of these patches to "Waiting on Author" and write a report >> (this one) with a CC to the authors. As all we know, we are short on >> reviewers and this action will save them a lot of time. Here [3] you >> can find a script I've been using to find such patches. >> >> I rechecked the list manually and did my best to exclude the patches >> that were updated recently or that depend on other patches. However >> there still a chance that your patch got to the list by a mistake. >> In this case please let me know.> > > With all due respect, it's hard not to see this as a disruption of the > current CF. I agree automating the patch processing is a worthwhile > goal, but we're not there yet and it seems somewhat premature. > > Let me explain why I think so: > > (1) You just changed the status of 10-15% open patches. I'd expect > things like this to be consulted with the CF manager, yet I don't see > any comments from Daniel. Considering he's been at the Oslo PUG meetup > today, I doubt he was watching hackers very closely. Correct, I’ve been travelling and running a meetup today so had missed this on -hackers. > (2) You gave everyone about 4 hours to object, ending 3PM UTC, which > excludes about one whole hemisphere where it's either too early or too > late for people to respond. I'd say waiting for >24 hours would be more > appropriate. Agreed. > I object to changing the patch status merely based on the script output. > It's a nice goal, but we need to do the legwork first, otherwise it'll > be just annoying and disrupting. I too fear that automating the state change will move patches away from “Needs review” in too many cases unless there is manual inspection step. Colliding on Oids in pg_proc comes to mind as a case where the patch won’t build, but the reviewer can trivially fix that locally and keep reviewing. > I suggest we inspect the reported patches manually, investigate whether > the failures are legitimate or not, and eliminate as many false > positives as possible. Once we are happy with the accuracy, we can > enable it again. This seems to summarize the sentiment in the other thread, this is absolutely a step in the right direction, we just need to tweak it with human knowledge before it can be made fully automatic to avoid false positives. The last thing we want is for the community to consider CF status changes/updates to be crying wolf, there are few enough reviewers as there is. cheers ./daniel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 13, 2017 at 7:39 AM, Daniel Gustafsson <daniel@yesql.se> wrote: >> On 12 Sep 2017, at 23:54, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote: >> With all due respect, it's hard not to see this as a disruption of the >> current CF. I agree automating the patch processing is a worthwhile >> goal, but we're not there yet and it seems somewhat premature. >> >> Let me explain why I think so: >> >> (1) You just changed the status of 10-15% open patches. I'd expect >> things like this to be consulted with the CF manager, yet I don't see >> any comments from Daniel. Considering he's been at the Oslo PUG meetup >> today, I doubt he was watching hackers very closely. > > Correct, I’ve been travelling and running a meetup today so had missed this on > -hackers. FWIW, I tend to think that the status of a patch ought to be changed by either a direct lookup at the patch itself or the author depending on how the discussion goes on, not an automatic processing. Or at least have more delay to allow people to object as some patches can be applied, but do not apply automatically because of naming issues. There are as well people sending test patches to allow Postgres to fail on purpose, for example see the replication slot issue not able to retain a past segment because the beginning of a record was not tracked correctly on the receiver-side. This can make the recovery tests fail, but we want them to fail to reproduce easily the wanted failure. >> (2) You gave everyone about 4 hours to object, ending 3PM UTC, which >> excludes about one whole hemisphere where it's either too early or too >> late for people to respond. I'd say waiting for >24 hours would be more >> appropriate. > > Agreed. Definitely. Any batch updates have to involve the CFM authorization at least, in this case Daniel. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
At Wed, 13 Sep 2017 08:13:08 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqTx=xq9XMqCgf9XEmq_PVEW99n6wjWDHi8aR3nnExyfGQ@mail.gmail.com> > On Wed, Sep 13, 2017 at 7:39 AM, Daniel Gustafsson <daniel@yesql.se> wrote: > >> On 12 Sep 2017, at 23:54, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote: > >> With all due respect, it's hard not to see this as a disruption of the > >> current CF. I agree automating the patch processing is a worthwhile > >> goal, but we're not there yet and it seems somewhat premature. > >> > >> Let me explain why I think so: > >> > >> (1) You just changed the status of 10-15% open patches. I'd expect > >> things like this to be consulted with the CF manager, yet I don't see > >> any comments from Daniel. Considering he's been at the Oslo PUG meetup > >> today, I doubt he was watching hackers very closely. > > > > Correct, I’ve been travelling and running a meetup today so had missed this on > > -hackers. > > FWIW, I tend to think that the status of a patch ought to be changed > by either a direct lookup at the patch itself or the author depending > on how the discussion goes on, not an automatic processing. Or at > least have more delay to allow people to object as some patches can be > applied, but do not apply automatically because of naming issues. > There are as well people sending test patches to allow Postgres to > fail on purpose, for example see the replication slot issue not able > to retain a past segment because the beginning of a record was not > tracked correctly on the receiver-side. This can make the recovery > tests fail, but we want them to fail to reproduce easily the wanted > failure. Agreed. I'd like to have a means to exclude a part of patches from the CI check. As another issue I can guess is that we sometimes fix the commit on which a patch(set) applies for a while especially for big patches, which gets many stubs continuously by new commits. > >> (2) You gave everyone about 4 hours to object, ending 3PM UTC, which > >> excludes about one whole hemisphere where it's either too early or too > >> late for people to respond. I'd say waiting for >24 hours would be more > >> appropriate. > > > > Agreed. > > Definitely. Any batch updates have to involve the CFM authorization at > least, in this case Daniel. +9(JST) regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Hello, aside from the discussion on the policy of usage of automation CI, it seems having trouble applying patches. https://travis-ci.org/postgresql-cfbot/postgresql/builds/274777750 >1363 heapam.c:2502:18: error: ‘HEAP_INSERT_SKIP_WAL’ undeclared (first use in this function) >1364 if (!(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation)) These lines shows that the patch is applied halfway. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Aleksander Alekseev wrote: > Agree, especially regarding build logs. All of this currently is only an > experiment. For some reason I got a weird feeling that at this time it > will be not quite successful one. If there will be too many false > positives I'll just return the patches back to "Needs Review" as it was > before. But I strongly believe that after a few iterations we will find > a solution that will be good enough and this slight inconveniences will > worth it in a long run. Thinking further, I think changing patch status automatically may never be a good idea; there's always going to be some amount of common sense applied beforehand (such as if a conflict is just an Oid catalog collision, or a typo fix in a recent commit, etc). Such conflicts will always raise errors with a tool, but would never cause a (decent) human being from changing the patch status to WoA. On the other hand, sending an email to the patch author (possibly CCing the mailing list, incl. In-Reply-To headers as appropriate) notifying them that there is a conflict would be very useful. I think it would be perfectly reasonable to automate that. Include exactly what went wrong, and of course the date where the problem was detected (in the email headers) so that reviewers can see "this patch's author received a notice and didn't act on it for two weeks" and take a decision to set it WoA. -- Álvaro Herrera https://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
Hi Tomas, I appreciate your feedback, although it doesn't seem to be completely fair. Particularly: > You gave everyone about 4 hours to object This is not quite accurate since my proposal was sent 2017-09-11 09:41:32 and this thread started - 2017-09-12 14:14:55. > You just changed the status of 10-15% open patches. I'd expect > things like this to be consulted with the CF manager, yet I don't see > any comments from Daniel. Agree, that was clearly a mistake, I had to add Daniel to CC. Sorry I didn't do that. I've returned all affected patches back to "Needs Review". On the bright side while doing this I've noticed that many patches were already updated by their authors. -- Best regards, Aleksander Alekseev
> On 13 Sep 2017, at 11:49, Aleksander Alekseev <a.alekseev@postgrespro.ru> wrote: > > Hi Tomas, > > I appreciate your feedback, although it doesn't seem to be completely > fair. I would like to stress one thing (and I am speaking only for myself here), this has been feedback and not criticism. Your (and everyone involved in this) initiative is great and automation will no doubt make the CF process better. We just need to finetune it a little to make it work for, as well as with, the community. cheers ./daniel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Hi Aleksander, On 09/13/2017 11:49 AM, Aleksander Alekseev wrote: > Hi Tomas, > > I appreciate your feedback, although it doesn't seem to be completely > fair. Particularly: > >> You gave everyone about 4 hours to object > > This is not quite accurate since my proposal was sent 2017-09-11 > 09:41:32 and this thread started - 2017-09-12 14:14:55. > Understood. I didn't really consider the first message to be a proposal with a deadline, as it starts with "here's a crazy idea" and it's not immediately clear that you intend to proceed with it immediately, and that you expect people to object. The message I referenced is a much clearer on this. >> You just changed the status of 10-15% open patches. I'd expect >> things like this to be consulted with the CF manager, yet I don't see >> any comments from Daniel. > > Agree, that was clearly a mistake, I had to add Daniel to CC. Sorry I > didn't do that. I've returned all affected patches back to "Needs > Review". On the bright side while doing this I've noticed that many > patches were already updated by their authors. > Yeah. regards -- Tomas Vondra 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
Hi Martin, > > === Build Failed: 7 === > > Title: Fix the optimization to skip WAL-logging on table created in same transaction > > Author: Martijn van Oosterhout <kleptog@svana.org> > > URL: https://commitfest.postgresql.org/14/528/ > > I'm not the author of this patch, and the page doesn't say so. > Something wrong with your script? It's a bug. The authors were determined by the senders of the first email in the corresponding thread. This is because I needed email list to CC the authors but the commitfest application doesn't show emails. Sorry for the disturbance. -- Best regards, Aleksander Alekseev
On Wed, Sep 13, 2017 at 5:18 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > Thinking further, I think changing patch status automatically may never > be a good idea; there's always going to be some amount of common sense > applied beforehand (such as if a conflict is just an Oid catalog > collision, or a typo fix in a recent commit, etc). Such conflicts will > always raise errors with a tool, but would never cause a (decent) human > being from changing the patch status to WoA. Well it would perhaps be fine if sending an updated patch bounced it right back to Needs Review. But if things are only being auto-flipped in one direction that's going to get tedious. Or one could imagine having the CF app show the CI status alongside the existing status column. -- 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
On Fri, Sep 15, 2017 at 8:13 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Sep 13, 2017 at 5:18 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: >> Thinking further, I think changing patch status automatically may never >> be a good idea; there's always going to be some amount of common sense >> applied beforehand (such as if a conflict is just an Oid catalog >> collision, or a typo fix in a recent commit, etc). Such conflicts will >> always raise errors with a tool, but would never cause a (decent) human >> being from changing the patch status to WoA. > > Well it would perhaps be fine if sending an updated patch bounced it > right back to Needs Review. But if things are only being auto-flipped > in one direction that's going to get tedious. > > Or one could imagine having the CF app show the CI status alongside > the existing status column. FWIW that last thing is the idea that I've been discussing with Magnus. That web page I made wouldn't exist, and the "apply" and "build" badges would appear directly on the CF app and you'd be able to sort and filter by them etc. The main submission status would still be managed by humans and the new apply and build statuses would be managed by faithful robot servants. How exactly that would work, I don't know. One idea is that a future cfbot, rewritten in better Python and adopted into the pginfra universe, pokes CF via an HTTPS endpoint so that the CF app finishes up with the information in its database. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers