+1
This was much needed :)
2015.01.07. 18:10 ezt írta ("Max Michels" <m...@data-artisans.com>):> Nice. > > > On Wed, Jan 7, 2015 at 5:51 PM, Ufuk Celebi <u...@apache.org> wrote: > > +1 > > > > @Stephan: thanks! :-) > > > > On Wed, Jan 7, 2015 at 4:44 PM, 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 > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> >
