Doerfert, Johannes via llvm-dev
2020-Jan-15 10:18 UTC
[llvm-dev] [PITCH] Improvements to LLVM Decision Making
On 01/15, James Henderson via llvm-dev wrote:> One other thought: any formal review period needs to be long enough for > people to contribute to if they have any annual leave from work or > whatever. For example, if the review period were to be set to two weeks, > I'd have missed proposals made at the start of roughly 2-3 different 2 week > periods last year. It would have been worse for 1 week. On the other hand, > a 3 week period would have meant I'd be able to read and respond to every > review. Note this is just an example - I'm not concretely suggesting 3 > weeks; perhaps it should be longer for bigger changes etc?There are various opinions on this (see for example the discussion here [0]). My take is that there is no fixed reasonable time to review and respond. There is a minimal one, due to weekends and time zones, but as soon as we take vacation/trips into account the problem is unbounded. Instead, I argue that post-reviews and potential revers are acceptable. If a consensus was reached and a reasonable* amount of time has passed changes should make it into the repository to guarantee timely progress for contributors. If problems are encountered later, either because the change was not on someones radar or because no one anticipated some problematic interaction, we should be flexible. A post-review discussion is appropriate if improvements are needed, a potential revert and follow-up review are appropriate if it was an actual breaking change. * Both "consensus" and "reasonable amount of time" are arguably vague here. Appropriate metrics depend on the impact of the proposed change and written guidelines would be helpful [1]. [0] http://lists.llvm.org/pipermail/llvm-dev/2019-November/136808.html [1] https://reviews.llvm.org/D71916 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200115/0dd7dd9e/attachment.sig>
James Henderson via llvm-dev
2020-Jan-15 11:58 UTC
[llvm-dev] [PITCH] Improvements to LLVM Decision Making
To be clear, my comment was purely intended to refer to non-technical decisions (i.e. not things like code reviews etc that stall because there's disagreement about what to do, but rather things like switching to the monorepo/github PRs etc). Indeed, my understanding from Chris's pitch is that his proposal isn't intended to address that either: " One note: While there are challenges with patch review and code owners, this proposal focuses on non-technical decisions that do not have a clear "code owner" escalation path today. This can include things like the introduction of new subprojects, introduction of a new social policies, change to core infrastructure like bug review tools or patch review processes, changes to the LLVM Developer Policy, etc." These decisions are significantly more impacting than the more technical ones as they usually impact pretty much every single developer. They are also often irreversible after a certain point, or at least would cause serious issues if we tried to reverse. Finally, once a decision has been made and started to be implemented, I always feel like there's a greater level required for objections, so people who weren't able to be involved are less likely to voice their opinions after the fact in a way that will actually generate any further discussion. Don't get me wrong, I agree that we can't keep a review open forever, since you can't accommodate everyone (e.g. months-long parental leave/long-term sicknesses/sabbaticals etc), but surely 1-2 weeks for such decisions isn't enough. On Wed, 15 Jan 2020 at 10:18, Doerfert, Johannes <jdoerfert at anl.gov> wrote:> On 01/15, James Henderson via llvm-dev wrote: > > One other thought: any formal review period needs to be long enough for > > people to contribute to if they have any annual leave from work or > > whatever. For example, if the review period were to be set to two weeks, > > I'd have missed proposals made at the start of roughly 2-3 different 2 > week > > periods last year. It would have been worse for 1 week. On the other > hand, > > a 3 week period would have meant I'd be able to read and respond to every > > review. Note this is just an example - I'm not concretely suggesting 3 > > weeks; perhaps it should be longer for bigger changes etc? > > There are various opinions on this (see for example the discussion here > [0]). > > My take is that there is no fixed reasonable time to review and respond. > There is a minimal one, due to weekends and time zones, but as soon as > we take vacation/trips into account the problem is unbounded. Instead, I > argue that post-reviews and potential revers are acceptable. If a > consensus was reached and a reasonable* amount of time has passed > changes should make it into the repository to guarantee timely progress > for contributors. If problems are encountered later, either because the > change was not on someones radar or because no one anticipated some > problematic interaction, we should be flexible. A post-review discussion > is appropriate if improvements are needed, a potential revert and > follow-up review are appropriate if it was an actual breaking change. > > > * Both "consensus" and "reasonable amount of time" are arguably > vague here. Appropriate metrics depend on the impact of the proposed > change and written guidelines would be helpful [1]. > > > [0] http://lists.llvm.org/pipermail/llvm-dev/2019-November/136808.html > [1] https://reviews.llvm.org/D71916 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200115/5d6fc61c/attachment.html>
Renato Golin via llvm-dev
2020-Jan-15 12:36 UTC
[llvm-dev] [PITCH] Improvements to LLVM Decision Making
On Wed, 15 Jan 2020 at 11:58, James Henderson via llvm-dev <llvm-dev at lists.llvm.org> wrote:> To be clear, my comment was purely intended to refer to non-technical decisions (i.e. not things like code reviews etc that stall because there's disagreement about what to do, but rather things like switching to the monorepo/github PRs etc). Indeed, my understanding from Chris's pitch is that his proposal isn't intended to address that either:High level technical also fall into that category, for example, changes in one area impacting others (like changing the IR semantics, pass manager structure, etc) as well as accepting new projects with varying degree of compatibility (like new front or back ends).> These decisions are significantly more impacting than the more technical ones as they usually impact pretty much every single developer. They are also often irreversible after a certain point, or at least would cause serious issues if we tried to reverse. Finally, once a decision has been made and started to be implemented, I always feel like there's a greater level required for objections, so people who weren't able to be involved are less likely to voice their opinions after the fact in a way that will actually generate any further discussion. Don't get me wrong, I agree that we can't keep a review open forever, since you can't accommodate everyone (e.g. months-long parental leave/long-term sicknesses/sabbaticals etc), but surely 1-2 weeks for such decisions isn't enough.I agree, that's why we need to be flexible on all counts. From understanding that people sometimes miss the time (and waiting for or pinging people), to accepting the responsibility of looking at those issues with high priority. First, we need to separate the noise, because the llvm-dev list is not suitable. I oftten only pick up when Alex's weekly comes up, sometimes it's too late. We need a clear and separate channel for decisions which can interrupt us with high priority. But also people that want/need to be involved in the decision process must be responsible for their own domain. For example asking someone else to look for important changes while you're away. cheers, --renato