I do think we need something less generic than our current @Experimental. I also like the idea of a separate package for unvetted contributions (though Incubating might simply be confusing, given that Apache uses Incubating for something else).
Good idea to have a standard way of marking such comments. On Tue, Feb 20, 2018 at 11:56 AM, Robert Bradshaw <[email protected]> wrote: > Thanks, Reuven, for bringing this up. This is an area well worth > investing time in. Specific comments below. > > On Mon, Feb 19, 2018 at 10:32 AM, Reuven Lax <[email protected]> wrote: > > > Pedantic > > > > Overly-pedantic comments (change variable names, etc.) can be > frustrating. > > The PR author can feel like they are being forced to make meaningless > > changes just so the reviewer will allow merging. Note that this is > sometimes > > in the eye of the beholder - the reviewer may not think all these > comments > > are pedantic. > > I think it would be good to have a convention for comments that are > suggestive, but not required. I usually prefix these with "Nit: ..." > Maybe "Optional: ..." would be better. This makes it clear that we're > not trying to put up unnecessary burdens for getting code in, but on > the other hand helps give guidance (which is often appreciated, > especially for newcomers (to the project, or coding in general)). For > any comment, one should be able to answer the question "why" and a > reviewer should feel free to ask this. > > > Don't Do This > > > > Sometimes a reviewer rejects an entire PR, saying that this should not be > > done. There are various reasons given: this won't scale, this will break > > backwards compatibility, this will break a specific runner, etc. The PR > > author may not always understand or agree with these reasons, and this > can > > leave hurt feelings. > > As mentioned, being able to point to a doc when issues like this arise > is a much better experience for the recipient. > > I like the PR dashboard idea as well. > > A separate package for less-well-vetted code is better than an > @Expiremental attribute. Incubating? Is there an expectation of > graduation at some point? >
