Date: Wed, 20 Jun 2018 13:07:28 -0700 From: Conrad Meyer <cem@freebsd.org> To: Jonathan Anderson <jonathan@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r335402 - head/sbin/veriexecctl Message-ID: <CAG6CVpX3cf-RvE%2B0vaUwhFEm=w7Dv-%2BVogSjWiVhDPsOC%2BAwJw@mail.gmail.com> In-Reply-To: <FAE743DA-A68B-4B52-A642-8B32AED79DE6@FreeBSD.org> References: <201806200108.w5K18sIR050132@repo.freebsd.org> <CAG6CVpV124ze%2BY6xX2ZFqbM%2B3hJNEJWR2qpnChpey=PmiW6qXg@mail.gmail.com> <96021.1529475664@kaos.jnpr.net> <CAJ5_RoBvwNH7-ZCd3LxtXg21TE49uX2y35Jwa6MM%2Bwn%2BX0_wUQ@mail.gmail.com> <17033.1529508519@kaos.jnpr.net> <CAG6CVpVwrWaDMcVRfgaOHagfPbnmULKe6R=GJiZi-reZYbZr8A@mail.gmail.com> <1529510299.24573.5.camel@freebsd.org> <CAG6CVpUgy8LhCkFZZ1D8BH%2BqJ_CDikvYNJrM9Nc0Ut5u=AVMHA@mail.gmail.com> <CAEm%2B2uVXQc7%2Bx6tmQyfeiU4rYKFMCcFZ2Q3_SHA1jf%2BOoHThfg@mail.gmail.com> <CADrOrmu=cQBNqU2_onT_s=pPhB0A546Nf9Sy5RcZzL0%2BAurpDQ@mail.gmail.com> <FAE743DA-A68B-4B52-A642-8B32AED79DE6@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Jon, On Wed, Jun 20, 2018 at 11:58 AM, Jonathan Anderson <jonathan@freebsd.org> wrote: > On 20 Jun 2018, at 15:32, Jonathan T. Looney wrote: >> My reasoning >> was fairly simple: when a review has been open for over a year with no >> action, it seems like the submitter should be able to commit it without >> waiting for more review, if they are confident in their change. I stand by >> that (and, in fact, would substitute something shorter for "over a year"). For this as a question of general process, I think "it depends." For minor fixes? Of course! Less than one year. For minor improvements or enhancements? Also yes, also probably less than 1yr. For large features like this, I think the answer is more nuanced. Sometimes being in a review for a year means the reviewers are overloaded and don't have time. In that case, maybe they shouldn't block progress. Sometimes being in review for a year just means the right reviewers haven't been found. I think that was more the case here. I would suggest anyone submitting a large feature and not getting feedback in a reasonable timeframe to solicit feedback from a wider audience much sooner than one year, and also indicate intention to merge the unreviewed change with some time window (1-2 weeks?) on giving feedback or at least asking for more time to review. > So: is there a strong reason why a backout-and-re-commit approach is > superior to iterating on the code in-tree? Yeah: > Any disputed change must be backed out pending resolution of the dispute > if requested by a maintainer. Security related changes may override a > maintainer's wishes at the Security Officer's discretion. > > This may be hard to swallow in times of conflict (when each side is > convinced that they are in the right, of course) but a version control > system makes it unnecessary to have an ongoing dispute raging when it is > far easier to simply reverse the disputed change, get everyone calmed > down again and then try to figure out what is the best way to proceed. > If the change turns out to be the best thing after all, it can be easily > brought back. If it turns out not to be, then the users did not have to > live with the bogus change in the tree while everyone was busily > debating its merits. People very rarely call for back-outs in the > repository since discussion generally exposes bad or controversial > changes before the commit even happens, but on such rare occasions the > back-out should be done without argument so that we can get immediately > on to the topic of figuring out whether it was bogus or not. In particular, SHA1 was never the primary reason the backout was requested, just one design smell in the aggregate. This change introduces a security feature that doesn't provide the security guarantees it claims to. That should be a major red flag. All the best, Conrad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpX3cf-RvE%2B0vaUwhFEm=w7Dv-%2BVogSjWiVhDPsOC%2BAwJw>