I think we should encourage contributors to supply design proposal before giving PR. When contributors have supplied their PRs ,they have implemented their codes by their original design ideas. Giving `Design Review` is usually too late to them ,and let them feel disappointed. So we should guide them to give design proposal thorough the dev emails , JIRAs or github issues.
On Sun, Oct 28, 2018 at 7:01 AM Jihoon Son <ghoon...@gmail.com> wrote: > Roman, thanks for the detailed explanation. I totally agree with it. > > I would add something more here for especially emphasizing the part of > 'design review'. > As Roman said, the PRs tagged with 'Design Review' usually include the > changes which are hard to undo, so, it's very important to make the best > decision we can. > And the best design decision can be found by only considering reasonable > evidences. > > So, I would suggest to enforce writing a proposal for all PRs which should > be labelled 'Design Review'. We're currently doing it only if someone wants > to write, but it gives a lot of benefits as below: > > - To write a detailed proposal, authors can have a chance to think about > their idea thoroughly including very details like APIs, configuration > names, or even some code level changes. > - We can have a chance that authors provide enough evidences for every > design decision, so that reviewers can check and verify them. I believe > that we can make the best decision in this process. > - Since it's only a proposal not containing any code changes yet, it's much > easier to change design decisions than changing codes. > > Many other open source projects are also doing this. For example, Kafka has > the 'Kafka Improvement Proposals' system ( > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals > ). > For Rust, it needs to write an RFC for substantial changes ( > http://rust-lang.github.io/rfcs/). > > I think one of the reasons why people hesitate to write proposals in Druid > is the absence of the appropriate format. I think the format should include > at least the followings: > > - The problem description and motivation > - Proposed change > - Overview of the change > - Details including public API changes, configuration changes, algorithm, > and so on > - Expected benefits and drawbacks > - Rationale and alternatives > > Welcome any idea. > > Best, > Jihoon > > On Fri, Oct 26, 2018 at 11:03 PM Roman Leventov <leven...@apache.org> > wrote: > > > Recently Charles (as far as I remember) asked what is the criteria for > > giving a PR 'Design Review' tag. > > > > I suggest that *a PR should be labelled "Design Review" if it will be > hard > > to undo it (after it appears in some release), it will have lasting > > consequences on the codebase.* > > > > Addition of a new config property, public API, or changing existing > public > > API is always hard to undo because it immediately becomes a subject for > > backwards compatibility. But some large code changes, even without > > user-facing API or config changes, could have similar effect on the > > codebase. > > > > Please keep in mind that it's not just for requiring two reviews instead > of > > one, but also for really reviewing the design. E. g. bad config > > key/property names could stay forever because it's really hard to rename > > them, even harder than change Java APIs. > > >
