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
