Just found out about this, thanks Stephan =) - Henry
On Wed, Jan 7, 2015 at 7:44 AM, Stephan Ewen <se...@apache.org> wrote: > Hi all! > > Since the feedback was positive, I added the guidelines to the wiki, with a > disclaimer that this is being refined. > > https://cwiki.apache.org/confluence/display/FLINK/Apache+Flink+development+guidelines > > Stephan > > > On Wed, Jan 7, 2015 at 4:13 PM, Kostas Tzoumas <ktzou...@apache.org> wrote: > >> +1 >> >> Let's encourage the use of component tags, I don't see the need for >> enforcing it. For commits that affect one component, I expect people will >> use it. >> >> On Wed, Jan 7, 2015 at 3:40 PM, Fabian Hueske <fhue...@apache.org> wrote: >> >> > +1 for the guide and JIRA references. >> > >> > I'd keep the component tags optional though. >> > As Max said, there is less space to display a meaning message if a commit >> > addresses several components. Separating changes into commits by >> components >> > sounds not very practical to me. >> > Also without a clear definition of when to add which component tag, we >> > cannot rely on them anyway. >> > >> > Git should also have better tools than browsing commit messages when >> > looking for a commit that changed specific code. >> > >> > 2015-01-07 15:24 GMT+01:00 Stephan Ewen <se...@apache.org>: >> > >> > > I personally like the tags very much. I think the streaming component >> was >> > > the first to introduce it and it stuck me as a very good idea. >> > > >> > > +1 to stick with them >> > > >> > > On Wed, Jan 7, 2015 at 3:03 PM, Márton Balassi < >> balassi.mar...@gmail.com >> > > >> > > wrote: >> > > >> > > > I prefer component declarations, the current best practice comes in >> > handy >> > > > when searching through commits. Answering a "when did key selection >> > > change >> > > > for streaming?" type question I just had to answer would have been a >> > bit >> > > > more difficult without it - manageable though. >> > > > >> > > > In case of streaming it does not yield much to omit the component >> > > > declaration, most of the time then we would need to add it to the >> > commit >> > > > message itself, e.g. : >> > > > "[streaming] Join API rework", could be e.g. rewritten as "Join API >> > > rework >> > > > for streaming". I do prefer the former one, because it is not only >> more >> > > > straight-forward to understand, but a bit shorter as well. >> > > > Of course there are counter-examples, like "[streaming] DataStream >> > > > refactor" -> "DataStream refactor". >> > > > >> > > > I can accept optional, but would like to keep it strongly recommended >> > for >> > > > streaming. I also find the [docs] tag helpful. >> > > > >> > > > On Wed, Jan 7, 2015 at 2:43 PM, Stephan Ewen <se...@apache.org> >> wrote: >> > > > >> > > > > Should we put that to an official vote, or wait for people to >> comment >> > > and >> > > > > (if nobody objects) consider it as agreed on through lazy >> consensus? >> > > > > >> > > > > On Wed, Jan 7, 2015 at 2:34 PM, Márton Balassi < >> > > balassi.mar...@gmail.com >> > > > > >> > > > > wrote: >> > > > > >> > > > > > +1 for the guide, thanks for clarifying the issue >> > > > > > >> > > > > > On Wed, Jan 7, 2015 at 2:30 PM, Till Rohrmann < >> > trohrm...@apache.org> >> > > > > > wrote: >> > > > > > >> > > > > > > +1 >> > > > > > > >> > > > > > > On Wed, Jan 7, 2015 at 12:41 PM, Aljoscha Krettek < >> > > > aljos...@apache.org >> > > > > > >> > > > > > > wrote: >> > > > > > > >> > > > > > > > Yes, we should have a guide like that somewhere. >> > > > > > > > >> > > > > > > > >> > > > > > > > On Wed, Jan 7, 2015 at 12:33 PM, Stephan Ewen < >> > se...@apache.org> >> > > > > > wrote: >> > > > > > > > >> > > > > > > > > We have not exactly defined this so far, but it is a good >> > point >> > > > to >> > > > > do >> > > > > > > so. >> > > > > > > > > >> > > > > > > > > I personally find it good to have changes associated with >> an >> > > > issue, >> > > > > > > > because >> > > > > > > > > it allows you to trace back why the change was done. >> > > > > > > > > To make sure we do not overdo this and impose totally >> > > unnecessary >> > > > > > > > overhead, >> > > > > > > > > I would suggest the following: >> > > > > > > > > >> > > > > > > > > *No issue is required for* >> > > > > > > > > >> > > > > > > > > - Small fixes like typos, simple warnings, >> > adding/improving a >> > > > > > comment >> > > > > > > > > >> > > > > > > > > - Adding and improving existing pages of the >> documentation >> > > > > > > > > >> > > > > > > > > - Simple improvements of style / elegance / efficiency >> > > (simple >> > > > > > > > rewriting >> > > > > > > > > a loop / condition / method interaction) if no behavior is >> > > > changed >> > > > > > > > > >> > > > > > > > > ==> Basically anything that does not change or add >> > > functionality >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > *An issue is required for* >> > > > > > > > > >> > > > > > > > > Everything else, in particular: >> > > > > > > > > >> > > > > > > > > - Anything that changes functionality or behavior >> relevant >> > to >> > > > > users >> > > > > > > > > >> > > > > > > > > - Anything that changes functionality or behavior >> relevant >> > to >> > > > > other >> > > > > > > > > components >> > > > > > > > > >> > > > > > > > > - Anything that adds a feature >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > I would vote to allow coarse issues and have multiple >> commits >> > > > that >> > > > > > > > > reference it >> > > > > > > > > >> > > > > > > > > [FLINK-1234] [runtime] Runtime support some cool new thing >> > > > > > > > > [FLINK-1234] [java api] Add hook for cool thing to java api >> > > > > > > > > [FLINK-1234] [scala api] Add hook for that thing to scala >> api >> > > > > > > > > [FLINK-1234] [optimizer] Make optimizer aware that it can >> > > exploit >> > > > > > this >> > > > > > > > > thing >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > ------------------------- >> > > > > > > > > >> > > > > > > > > The guide lines for pull-requests for committers are as >> > > follows: >> > > > > > > > > >> > > > > > > > > *A pull request with comments/additional signoff is >> required >> > > for >> > > > > > > anything >> > > > > > > > > that* >> > > > > > > > > >> > > > > > > > > - breaks the public APIs >> > > > > > > > > >> > > > > > > > > - adds methods to the public APIs (that will need to be >> > kept >> > > > > stable >> > > > > > > > from >> > > > > > > > > them on) >> > > > > > > > > >> > > > > > > > > - alters user-facing behavior (e.g., mutability of types, >> > > null >> > > > > > value >> > > > > > > > > handling, window semantics, ...) >> > > > > > > > > >> > > > > > > > > - adds user-facing knobs (switches, config parameters, >> > > > execution >> > > > > > > option >> > > > > > > > > on the execution environment) >> > > > > > > > > >> > > > > > > > > - adds additional maven dependencies >> > > > > > > > > >> > > > > > > > > - changes the way components interact >> > > > > > > > > >> > > > > > > > > - touches highly sensitive and performance critical >> parts, >> > > such >> > > > > > > memory >> > > > > > > > > management or network stack >> > > > > > > > > >> > > > > > > > > ==> Changes that come with a pull request should have one >> or >> > > more >> > > > > > > issues >> > > > > > > > > associated with them. >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > Anyone that wants to have comments or some additional pairs >> > of >> > > > eyes >> > > > > > in >> > > > > > > > the >> > > > > > > > > code should make a pull request as well. >> > > > > > > > > >> > > > > > > > > ------------------------- >> > > > > > > > > >> > > > > > > > > *Naming scheme for commits* >> > > > > > > > > >> > > > > > > > > [issue] [component] Message >> > > > > > > > > >> > > > > > > > > For fixes without an issue, the issue can be dropped. >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > What do you think? Should we put this into the Wiki? >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > Greetings, >> > > > > > > > > Stephan >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > On Wed, Jan 7, 2015 at 11:48 AM, Aljoscha Krettek < >> > > > > > aljos...@apache.org >> > > > > > > > >> > > > > > > > > wrote: >> > > > > > > > > >> > > > > > > > > > Hi, >> > > > > > > > > > I feel we never really talked about this. So, should we >> > open >> > > > Jira >> > > > > > > > issues >> > > > > > > > > > even for very small fixes and then add the ticket number >> to >> > > the >> > > > > > > commit? >> > > > > > > > > Or >> > > > > > > > > > should we just commit those small fixes. Right now, I >> have >> > > two >> > > > > > small >> > > > > > > > > fixes >> > > > > > > > > > (one is 4 lines, the other one is two lines) for the >> > > > > ValueTypeInfo >> > > > > > > and >> > > > > > > > > > TextValueInputFormat. Very obscure stuff, I know. :D >> > > > > > > > > > >> > > > > > > > > > Cheers, >> > > > > > > > > > Aljoscha >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >>
