Statically, yes, GitHub PRs are the same as GitHub cases. But dynamically, they are different, because you can only log a PR when you have finished work.
A lot of other Apache projects use JIRA, so there is a clear distinction between cases and contributions. JIRA cases, especially when logged early in the lifecycle of a contribution, become long-running conversation threads with a lot of community participation. If the Druid chose to do so, GitHub cases could be the same. Be careful that you do not treat “potential contributors” (by which I presume you mean non-committers) differently from committers and PMC members. Anyone starting a major piece of work should follow the same process. (Experienced committers probably have a somewhat better idea what work will turn out to be “major”, so they get a little more leeway.) Julian > On Jan 7, 2019, at 12:10 PM, Gian Merlino <g...@apache.org> wrote: > > I don't think there's a need to raise issues for every change: a small bug > fix or doc fix should just go straight to PR. (GitHub PRs show up as issues > in the issue-search UI/API, so it's not like this means the patch has no > corresponding issue -- in a sense the PR _is_ the issue.) > > I do think it makes sense to encourage potential contributors to write to > the dev list or raise an issue if they aren't sure if something would need > to go through a more heavy weight process. > > Fwiw we do have a set of 'design review' criteria already (we had a > discussion about this a couple years ago) at: > http://druid.io/community/#getting-your-changes-accepted. So we wouldn't be > starting from zero on defining that. We set it up back when we were trying > to _streamline_ our process -- we used to require two non-author +1s for > _every_ change, even minor ones. The introduction of design review criteria > was meant to classify which PRs need that level of review and which ones > are minor and can be merged with less review. I do think it helped with > getting minor PRs merged more quickly. The list of criteria is, > > - Major architectural changes or API changes > - HTTP requests and responses (e. g. a new HTTP endpoint) > - Interfaces for extensions > - Server configuration (e. g. altering the behavior of a config property) > - Emitted metrics > - Other major changes, judged by the discretion of Druid committers > > Some of it is subjective, but it has been in place for a while, so it's at > least something we are relatively familiar with. > > On Mon, Jan 7, 2019 at 11:32 AM Julian Hyde <jh...@apache.org> wrote: > >> Small contributions don’t need any design review, whereas large >> contributions need significant review. I don’t think we should require an >> additional step for those (many) small contributions. But who decides >> whether a contribution fits into the small or large category? >> >> I think the solution is for authors to log a case (or send an email to >> dev) before they start work on any contribution. Then committers can >> request a more heavy-weight process if they think it is needed. >> >> Julian >> >> >>> On Jan 7, 2019, at 11:24 AM, Gian Merlino <g...@apache.org> wrote: >>> >>> It sounds like splitting design from code review is a common theme in a >> few >>> of the posts here. How does everyone feel about making a point of >>> encouraging design reviews to be done as issues, separate from the pull >>> request, with the expectations that (1) the design review issue >>> ("proposal") should generally appear somewhat _before_ the pull request; >>> (2) pull requests should _not_ have design review happen on them, meaning >>> there should no longer be PRs with design review tags, and we should move >>> the design review approval process to the issue rather than the PR. >>> >>> For (1), even if we encourage design review discussions to start before a >>> pull request appears, I don't see an issue with them running concurrently >>> for a while at some point. >>> >>> On Thu, Jan 3, 2019 at 5:35 PM Jonathan Wei <jon...@apache.org> wrote: >>> >>>> Thanks for raising these concerns! >>>> >>>> My initial thoughts: >>>> - I agree that separation of design review and code-level review for >> major >>>> changes would be more efficient >>>> >>>> - I agree that a clear, more formalized process for handling major >> changes >>>> would be helpful for contributors: >>>> - Define what is considered a major change >>>> - Define a standard proposal structure, KIP-style proposal format >> sounds >>>> good to me >>>> >>>> - I think it's too rigid to have a policy of "no code at all with the >>>> initial proposal" >>>> - Code samples can be useful references for understanding aspects of a >>>> design >>>> - In some cases it's necessary to run experiments to fully understand a >>>> problem and determine an appropriate design, or to determine whether >>>> something is even worth doing before committing to the work of fleshing >> out >>>> a proposal, prototype code is a natural outcome of that and I'm not >> against >>>> someone providing such code for reference >>>> - I tend to view design/code as things that are often developed >>>> simultaneously in an intertwined way >>>> >>>>> Let's not be naive this is very rare that a contributor will accept >> that >>>> his work is to be thrown, usually devs takes coding as personal creation >>>> and they get attached to it. >>>> >>>> If we have a clear review process that emphasizes the need for early >>>> consensus building, with separate design and code review, then I feel >> we've >>>> done enough and don't need a hard rule against having some code linked >> with >>>> the initial proposal. If a potential contributor then still wants to go >>>> ahead and write a lot of code that may be rejected or change >> significantly, >>>> the risks were made clear. >>>> >>>>> Once code is written hard to think abstract. >>>> >>>> I can see the validity of the concern, but I personally don't see it as >> a >>>> huge risk. My impression from the Druid PR reviews I've seen is that our >>>> reviewers are able to keep abstract design vs. implementation details >>>> separate and consider alternate designs when reviewing. >>>> >>>> To summarize I think it's probably enough to have a policy along the >> lines >>>> of: >>>> - Create more formalized guidelines for proposals and what changes >> require >>>> proposals >>>> - Separate design and code review for major changes, with design review >>>> first, code-level review after reaching consensus on the design. >>>> - Code before the design review is completed is just for reference, not >>>> regarded as a candidate for review/merging. >>>> >>>> - Jon >>>> >>>> >>>> On Thu, Jan 3, 2019 at 12:48 PM Slim Bouguerra < >> slim.bougue...@gmail.com> >>>> wrote: >>>> >>>>> On Thu, Jan 3, 2019 at 12:16 PM Clint Wylie <clint.wy...@imply.io> >>>> wrote: >>>>> >>>>>> I am definitely biased in this matter as an owner of another large PR >>>>> that >>>>>> wasn't preceded by a direct proposal or dev list discussion, and in >>>>> general >>>>>> I agree that proposal first is usually better, but I think in some >>>> rarer >>>>>> cases approaching a problem code first *is* the most appropriate way >> to >>>>>> have a discussion. >>>>> >>>>> >>>>> I am wondering here what is the case where code first is better? >>>>> In general when you are writing code you have an idea about what you >> want >>>>> to change, why you want to change and why you want to change it. >>>>> I do not see what is wrong with sharing this primitive ideas and >> thoughts >>>>> as an abstract proposal (at least to avoid overlapping) >>>>> >>>>> I see nothing wrong with it so long as the author >>>>>> accepts that the PR is treated as a combined proposal and proof of >>>>> concept, >>>>>> and fair game to be radically changed via discussion or even rejected, >>>>>> which sounds like Gian's attitude on the matter and is mine as well >>>> with >>>>> my >>>>>> compression stuff. >>>>> >>>>> >>>>> Let's not be naive this is very rare that a contributor will accept >> that >>>>> his work is to be thrown, usually devs takes coding as personal >> creation >>>>> and they get attached to it. >>>>> To my point you can take a look on some old issue in the Druid forum >>>>> >>>> >> https://github.com/apache/incubator-druid/pull/3755#issuecomment-265667690 >>>>> and am sure other communities have similar problems. >>>>> So leaving the door open to some side cases is not a good idea in my >>>>> opinion and will lead to similar issue in the future. >>>>> >>>>> This seems to me especially likely to happen in cases >>>>>> where an approach still needs proven to be a viable idea *to the >>>> author*, >>>>>> so that a much more productive discussion can be had in the first >>>> place. >>>>>> >>>>>> I think there is a trade off, I don't think we want to discourage >>>>>> experimentation by walling it off behind mandatory discussions before >>>> it >>>>>> can even start, but I do think formalizing the process for large >>>> changes >>>>> is >>>>>> a good thing, especially since we probably can't expect the wider >>>>> community >>>>>> to have the same attitude towards a large PR getting discarded as a >>>>>> committer might. I think the Kafka approach is reasonable, a bit more >>>>>> formal than our design review process but not overbearing. >>>>> >>>>> >>>>> Can you please explain what is overbearing ? what can be changed to >> make >>>> it >>>>> easy ? >>>>> Most of the points are kind of the actual questions that you want to >>>>> address before hand anyway isn't ? >>>>> >>>>> >>>>>> Going code first >>>>>> should be in general discouraged, but when it does happen, it seems >>>> like >>>>>> opening DIP/an issue/starting a mailing list thread or whatever we go >>>>> with >>>>>> to have a more high level design discussion alongside the reference PR >>>>>> could alleviate some of these complaints? >>>>> >>>>> >>>>> What are the complaints ? >>>>> >>>>> >>>>>> +1 for "DIP" heh, I think making >>>>>> them in the form of github issues is probably appropriate, with a dev >>>>> list >>>>>> thread to announce them perhaps? >>>>>> >>>>> >>>>> I think github issue with [Proposal] header like >>>>> https://github.com/apache/incubator-druid/issues/4349 is good to me, >>>>> >>>>> Thanks! >>>>> >>>>> >>>>>> On Thu, Jan 3, 2019 at 10:28 AM Slim Bouguerra <bs...@apache.org> >>>> wrote: >>>>>> >>>>>>> Thanks everyone for interacting with this thread. >>>>>>> >>>>>>> The fact that i, Roman, Jihoon and others in the past (FJ >>>>>>> >>>>>> >>>>> >>>> >> https://groups.google.com/forum/#!msg/druid-user/gkUEsAYIfBA/6B2GJdLkAgAJ) >>>>>>> raised this point indicates that PRs without a proposal are indeed an >>>>>> issue >>>>>>> and we need to solve it. >>>>>>> >>>>>>> Something Similar to KIP maybe called DIPs is fine with me. >>>>>>> What i strive to see is the following: >>>>>>> >>>>>>> [Step 1] formalize what is the kind of work that needs a formal >>>>>> Proposal, I >>>>>>> think Roman and Jihoon has already covered that pretty well. am +1 on >>>>>> that. >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >> https://lists.apache.org/thread.html/9b30d893bdb6bb633cf6a9a700183ffb5b98f115330531a55328ac77@%3Cdev.druid.apache.org%3E >>>>>>> >>>>>>> I am strongly in favor of the separation of Proposal Review and >>>> (later) >>>>>>> Code review PRs. My main reasons: >>>>>>> Most importantly code reviewing will introduce lot of noise and will >>>>>>> ultimately make the GitHub page unreadable. >>>>>>> Avoid overlapping of work. >>>>>>> Once code is written hard to think abstract. >>>>>>> Separate page for Design review later can always be used it as a >>>> Design >>>>>>> document that is readable and code free-ish. >>>>>>> As i said the goal of this first round is to see if the community >>>> agree >>>>>>> about such change, then make the process of design more inclusive >>>> thus >>>>>>> other contributors can submit a counter proposals. >>>>>>> >>>>>>> [Step 2] IF everybody agree about that point Step 2 is to define >>>> which >>>>>>> medium is used to Publish a primitive form of a CODE FREE Abstract >>>>>> Proposal >>>>>>> containing at least the following bullet points. >>>>>>> - The problem description and motivation >>>>>>> - Overview of the proposed change >>>>>>> - Operational impact (compatibility/ plans to upgrades) public API >>>>>> changes, >>>>>>> configuration changes, algorithm, and so on >>>>>>> - Expected benefits and drawbacks >>>>>>> - Rationale and alternatives >>>>>>> - Estimate Time to Deliver if possible. >>>>>>> >>>>>>> The way i think this can be is a Github issue where member of the >>>>>> community >>>>>>> will interact via comments and the author will be updating the >>>>>> description >>>>>>> in the light of comments provided by the community. >>>>>>> >>>>>>> During and near the end of the design discussions the author/s can >>>>> start >>>>>>> writing POCs to help guide the review process this naturally will be >>>> a >>>>>> Pull >>>>>>> request with actual code. >>>>>>> >>>>>>> *Now the most important thing is that we need to agree that any work >>>>> that >>>>>>> does not align with this formal process will be ignored and the >>>> author >>>>>> will >>>>>>> be asked to start with a DIP* >>>>>>> *That is what i meant with “If it didn’t happen on the mailing list, >>>>> it >>>>>>> didn’t happen.”* >>>>>>> >>>>>>> Thanks and happy coding! >>>>>>> >>>>>>> On Wed, Jan 2, 2019 at 6:47 PM Gian Merlino <g...@apache.org> wrote: >>>>>>> >>>>>>>> One of the advantages I see with a more formal process is (like >>>> Kafka >>>>>>> KIPs) >>>>>>>> is that it levels the playing field a bit and sets some ground >>>> rules >>>>>> for >>>>>>>> working together. In a way it can help encourage contributions by >>>>>> making >>>>>>> it >>>>>>>> clear what is expected of potential contributors. >>>>>>>> >>>>>>>> We have a design review process today that is not as formal as >>>> KIPs, >>>>>> but >>>>>>> is >>>>>>>> somewhat heavier than the one you describe. Maybe we could tweak >>>> our >>>>>>>> current one by starting to do design reviews separately from PRs. >>>>> i.e., >>>>>>> for >>>>>>>> anything that meets our 'design review' criteria, do that on the >>>> dev >>>>>> list >>>>>>>> or in a separate issue, and keep the PR focused on code-level >>>> stuff. >>>>>> That >>>>>>>> way we don't end up trying to do both at once. And it makes it >>>> easier >>>>>> to >>>>>>>> start talking about design before the code is ready, which would be >>>>>>> better. >>>>>>>> >>>>>>>> On Wed, Jan 2, 2019 at 6:10 PM Julian Hyde <jh...@apache.org> >>>> wrote: >>>>>>>> >>>>>>>>> It’s really hard to say no to a contribution when someone has put >>>>> in >>>>>> a >>>>>>>>> significant amount of work. >>>>>>>>> >>>>>>>>> The following approach is simple and works really well: Before >>>> you >>>>>>> start >>>>>>>>> work, log a case, describing the problem. When you have some >>>> ideas >>>>>>> about >>>>>>>>> design, add those to the case. When you have a code branch, add >>>> its >>>>>> URL >>>>>>>> to >>>>>>>>> the case. And so forth. At any point in the proceedings, people >>>> can >>>>>>> chime >>>>>>>>> in with their opinions. >>>>>>>>> >>>>>>>>> In my opinion, a formal “design review” process is not necessary. >>>>>> Just >>>>>>>>> build consensus iteratively, by starting the conversation early >>>> in >>>>>> the >>>>>>>>> process. >>>>>>>>> >>>>>>>>> Julian >>>>>>>>> >>>>>>>>> >>>>>>>>>> On Jan 2, 2019, at 12:37 PM, Gian Merlino <g...@apache.org> >>>>> wrote: >>>>>>>>>> >>>>>>>>>> In this particular case: please consider the PR as a proposal. >>>>>> Don't >>>>>>>> feel >>>>>>>>>> like just because there is code there that takes a certain >>>>>> approach, >>>>>>>> that >>>>>>>>>> the approach is somehow sacred. I had to implement something to >>>>>>>>> crystallize >>>>>>>>>> my own thinking about how the problem could be approached. I >>>>> won't >>>>>> be >>>>>>>>>> disappointed if, as a community, we decide a different >>>> direction >>>>> is >>>>>>>>> better >>>>>>>>>> and the code all gets thrown away. That's one of the reasons >>>>> that I >>>>>>>>> removed >>>>>>>>>> the 0.14.0 milestone that was added to the patch. (I don't want >>>>> to >>>>>>> rush >>>>>>>>> it, >>>>>>>>>> nor do I think that's a good idea.) >>>>>>>>>> >>>>>>>>>> In general: Sounds like we could do with some more >>>> formalization >>>>>>> around >>>>>>>>>> what a proposal looks like, which sorts of changes need one, >>>> and >>>>>> when >>>>>>>> in >>>>>>>>>> the dev cycle it is appropriate. FWIW I think Kafka's process >>>> is >>>>>> more >>>>>>>> or >>>>>>>>>> less fine, and would be okay with adopting it for Druid if >>>> people >>>>>>> like >>>>>>>>> it. >>>>>>>>>> Right now our standards for what requires a "design review" are >>>>>> very >>>>>>>>>> similar to the Kafka community standards for what requires a >>>> KIP, >>>>>> so >>>>>>> we >>>>>>>>>> have some familiarity with those concepts. However we don't >>>>>> separate >>>>>>> PR >>>>>>>>>> review and proposal discussion as strictly as they do, which >>>>> seems >>>>>> to >>>>>>>> be >>>>>>>>>> the foundation for the feeling of exclusion that is being felt >>>>>> here. >>>>>>>>>> >>>>>>>>>> Separately: I just redid the description on >>>>>>>>>> https://github.com/apache/incubator-druid/pull/6794 to be more >>>>>>>>> proposal-y. >>>>>>>>>> I followed the KIP style: >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals >>>>>>>>> . >>>>>>>>>> Please refresh the page and see if it looks more useful. >>>>>>>>>> >>>>>>>>>> Gian >>>>>>>>>> >>>>>>>>>> On Wed, Jan 2, 2019 at 10:52 AM Julian Hyde <jh...@apache.org> >>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Slim, >>>>>>>>>>> >>>>>>>>>>> I agree with your points that offline development is bad for >>>>>>>> community. >>>>>>>>>>> But I don’t think you need much mentor help. You have raised >>>>> valid >>>>>>>>> issues >>>>>>>>>>> and the Druid community needs to decide what its development >>>>>>> practices >>>>>>>>>>> should be. >>>>>>>>>>> >>>>>>>>>>> Julian >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On Jan 2, 2019, at 10:29 AM, Slim Bouguerra < >>>> bs...@apache.org> >>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>> Hello everyone and hope you all have very good holidays. >>>>>>>>>>>> >>>>>>>>>>>> First, this email is not directed on the author or the PR >>>>>>>>>>>> https://github.com/apache/incubator-druid/pull/6794 it >>>> self, >>>>>> but >>>>>>> i >>>>>>>>> see >>>>>>>>>>>> this PR as a perfect example. >>>>>>>>>>>> >>>>>>>>>>>> One of the foundation of Apache Way or what i would simply >>>> call >>>>>>> open >>>>>>>>>>> source >>>>>>>>>>>> community driven development is that "Technical decisions are >>>>>>>>> discussed, >>>>>>>>>>>> decided, and archived publicly. >>>>>>>>>>>> developpement" >>>>>>>>>>>> Which means that big technical changes such as the one >>>> brought >>>>>> by >>>>>>>>> #/6794 >>>>>>>>>>>> should have started as a proposal and round of discussions >>>>> about >>>>>>> the >>>>>>>>>>> major >>>>>>>>>>>> changes designs not as 11K line of code. >>>>>>>>>>>> I believe such openness will promote a lot of good benefits >>>>> such >>>>>>> as: >>>>>>>>>>>> >>>>>>>>>>>> - ensures community health and growth. >>>>>>>>>>>> - ensures everyone can participate not only the authors and >>>> his >>>>>>>>>>> co-workers. >>>>>>>>>>>> - ensures that the project is driven by the community and >>>> not a >>>>>>> given >>>>>>>>>>>> company or an individual. >>>>>>>>>>>> - ensures that there is consensus (not saying 100% >>>> agreement;) >>>>>>>> however >>>>>>>>> it >>>>>>>>>>>> means that all individuals will accept the current progress >>>> on >>>>>> the >>>>>>>>>>> project >>>>>>>>>>>> until some better proposal is put forth. >>>>>>>>>>>> >>>>>>>>>>>> Personally such BIG offline PR makes me feel excluded and >>>>> doesn't >>>>>>>> give >>>>>>>>>>> me a >>>>>>>>>>>> sense that i belong to a community at all. >>>>>>>>>>>> >>>>>>>>>>>> To prevent such off list development i think as a Druid >>>>> Community >>>>>>> we >>>>>>>>> need >>>>>>>>>>>> to stick to the apache way “If it didn’t happen on the >>>> mailing >>>>>>> list, >>>>>>>> it >>>>>>>>>>>> didn’t happen.” >>>>>>>>>>>> >>>>>>>>>>>> I would appreciate if some of the Apache mentor help with >>>> this. >>>>>>>>>>>> Thanks >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org >>>>>>>>>>> For additional commands, e-mail: dev-h...@druid.apache.org >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>> --------------------------------------------------------------------- >>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org >>>>>>>>> For additional commands, e-mail: dev-h...@druid.apache.org >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> >>>>> B-Slim >>>>> >> _______/\/\/\_______/\/\/\_______/\/\/\_______/\/\/\_______/\/\/\_______ >>>>> >>>> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org >> For additional commands, e-mail: dev-h...@druid.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@druid.apache.org For additional commands, e-mail: dev-h...@druid.apache.org
