I'm fine moving forward with this. My vote is -0, but I would not block it.
However, I think it is important to raise objections now so that we are all clear that this is a guideline to set expectations for contributors. It is not a rule that tells committers how to act. On Fri, Aug 2, 2024 at 12:08 PM Walaa Eldin Moustafa <wa.moust...@gmail.com> wrote: > My concern with this change (in its current form) is that it combines > (mixes?) three things: > > [1] A few paragraphs/statements that delegate to the comitter's > judgement call. (e.g., "Committer is trusted", "If committer feels" > ..). So the value of adding them is not very clear to me. > [2] A few things that are implied (e.g., re-iterating ASF guidelines, > lack of agreement will definitely lead to not merging the PR -- > whether to discuss the topic as a proposal or not should have a > separate process to state what qualifies as a proposal discussion). > [3] A few crisp rules, e.g, how to treat changes under spec and files > under the `format` directory. > > I would avoid [1] and [2] to avoid repetition or too much reading of > those rules, especially when they are subjective (e.g., [1]) or > implied (e.g., [2]). What remains (e.g., [3] or the way to proceed if > a committer feels something is worthy of a proposal-level discussion) > fits more in a process that organizes what qualifies as a proposal vs > code change etc (e.g., the rule should not be about "whether a > committer feels that something is worth a proposal", but rather there > should be a process that guides both the contributor and committer for > what qualifies as proposal vs a direct PR). > > Thanks, > Walaa. > > > On Fri, Aug 2, 2024 at 11:43 AM Micah Kornfield <emkornfi...@gmail.com> > wrote: > > > > I just wanted to follow up on this. > > > > Ryan and Szehon unless you strongly object to the current formulation of > the PR, I'd like to move forward with a vote to merge. There are already a > fair number of people in the community that have reviewed and seemed to > think the proposal is not unreasonable. For votes it seems like most of > the recent threads on the matter have decided to err on the side of votes > for changes to specs, as it reduces the judgement call of reviewers. > Thoughts? > > > > Thanks, > > Micah > > > > On Tue, Jul 30, 2024 at 11:08 AM Micah Kornfield <emkornfi...@gmail.com> > wrote: > >>> > >>> The problem I'm worried about is the tendency to misuse docs like this > and become focused on it as a rule. People tend to apply written rules > mechanically and I worry about people substituting a reading of this text > for judgement. For example, a strict reading of "encouraged to ask another > committer" means that it is optional. > >> > >> > >> I agree with this concern which is why I tried to make everything > optional. I hope if there ever needs to be a revision here it is framed as > constructive and not a blame game on someone breaking the rules. In > general, I think placing the reminder on trust and putting the project > first are good reminders in either case. IMO the text is short enough that > they should be included directly in the contributors guideline. > >> > >> > >>> To me, (2) is a bit new here and in the grey area for interpretation. > I was thinking about this while reviewing > https://github.com/apache/iceberg/pull/10793, which could be a category > (2) and non-functional change but would need a full code-modification vote > as per [Iceberg improvement > proposal](#apache-iceberg-improvement-proposals). I can see both sides, to > avoid a potential dispute/misunderstanding over the clarification, it would > be nice to have a vote on the devlist. But it may also be yet another > burden, when something can be more easily decided on the github discussion > itself via approval by the relevant parties. > >> > >> > >> The PR documents how I understood the requirement communicated to me > about spec modifications. I don't recall seeing exactly where this > requirement was agreed. so maybe we can revise it. IMO spec changes are > important enough that it is worth explicit email to the mailing list > notifying people of the proposed change. A vote might be too heavyweight > though. Would updating the guideline to require a separate email to the > mailing list for changes and requiring the change be open for at least 72 > hours strike the right balance? > >> > >> Thanks, > >> Micah > >> > >> > >> > >> > >> > >> > >> On Mon, Jul 29, 2024 at 3:08 PM Szehon Ho <szehon.apa...@gmail.com> > wrote: > >>> > >>> Typo , wrong link: > >>> (2) requiring full code-modification vote as per [Iceberg improvement > proposal](#apache-iceberg-improvement-proposals) => full code-modification > vote as per [code modification]( > https://www.apache.org/foundation/voting.html#votes-on-code-modification) > >>> > >>> On Mon, Jul 29, 2024 at 1:53 PM Szehon Ho <szehon.apa...@gmail.com> > wrote: > >>>> > >>>> Hi, > >>>> > >>>> Also if I read it correctly, I think this proposal imposes the > following workflows in "spec" folders : > >>>> > >>>> Large and functional changes. These redirect to Iceberg improvement > proposals, which ends in code-modification vote > >>>> bug-fixes or clarification, which is specified to require > code-modification vote > >>>> grammar, spelling, minor formatting fix, not covered here (I guess it > is like any normal code review?) > >>>> > >>>> To me, (2) is a bit new here and in the grey area for > interpretation. I was thinking about this while reviewing > https://github.com/apache/iceberg/pull/10793, which could be a category > (2) and non-functional change but would need a full code-modification vote > as per [Iceberg improvement > proposal](#apache-iceberg-improvement-proposals). I can see both sides, to > avoid a potential dispute/misunderstanding over the clarification, it would > be nice to have a vote on the devlist. But it may also be yet another > burden, when something can be more easily decided on the github discussion > itself via approval by the relevant parties. So I think I would agree with > Ryan in mentioning that significant (would maybe add "functional") spec > change needs a vote on the dev list. > >>>> > >>>> Thanks > >>>> Szehon > >>>> > >>>> On Mon, Jul 29, 2024 at 1:16 PM Ryan Blue <b...@databricks.com.invalid> > wrote: > >>>>> > >>>>> I think the proposed doc looks good, but I'm not sure that it is > better to add this to our guidelines. > >>>>> > >>>>> On one hand the doc describes how ASF communities work in general: > committers review and commit PRs and are expected to use good judgement, > ask one another for help when necessary, and broaden the set of people in > the discussion when there's a disagreement. I really appreciate that Micah > called out that this is intentionally vague to emphasize committer > judgement. > >>>>> > >>>>> The problem I'm worried about is the tendency to misuse docs like > this and become focused on it as a rule. People tend to apply written rules > mechanically and I worry about people substituting a reading of this text > for judgement. For example, a strict reading of "encouraged to ask another > committer" means that it is optional. > >>>>> > >>>>> Given that the majority of the content here is stating how ASF > communities work and the only Iceberg-specific parts are the proposal > process and calling out that we vote on spec changes, I would probably just > have a description of how to handle proposals (which is already there) and > a note that significant spec changes should use a vote on the dev list. > >>>>> > >>>>> On Sun, Jul 28, 2024 at 11:15 PM Jean-Baptiste Onofré < > j...@nanthrax.net> wrote: > >>>>>> > >>>>>> Hi Micah > >>>>>> > >>>>>> Thanks ! It looks good to me now you have included comments from > everyone. > >>>>>> > >>>>>> Regards > >>>>>> JB > >>>>>> > >>>>>> On Fri, Jul 26, 2024 at 1:15 AM Micah Kornfield < > emkornfi...@gmail.com> wrote: > >>>>>> > > >>>>>> > As part of the bylaws discussions that have been happening, we > are trying to make small focused proposals to move things forward. As a > first step towards this I created a proposal for guidelines on committing > pull requests [1]. Feedback is appreciated. > >>>>>> > > >>>>>> > Given the level of interest in the discussions so far, it seems > that the best path forward is to hold an official vote before merging. I > intend to do this once we appear to have consensus but if the people prefer > we can try to avoid the overhead. > >>>>>> > > >>>>>> > Thanks, > >>>>>> > Micah > >>>>>> > > >>>>>> > > >>>>>> > [1] https://github.com/apache/iceberg/pull/10780 > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Ryan Blue > >>>>> Databricks > -- Ryan Blue Databricks
