I did not propose to pick the 5 most suitable tags. ;-) Just to mark commits that break or extend the API. If [api-breaking] and [api-extending] is considered to waste too much space, I would also be happy with [api-] and [api+] (or any other meaningful marker).
2015-01-28 2:37 GMT+01:00 Márton Balassi <balassi.mar...@gmail.com>: > Originally I was going to support Fabian's suggestion - I still like it, > but I have to admit that it can become a slight overkill like in de8e066. > [1] > > [1] https://git-wip-us.apache.org/repos/asf/flink/commit/de8e066 > > On Tue, Jan 27, 2015 at 2:03 PM, Max Michels <m...@data-artisans.com> > wrote: > > > I agree with Aljoscha. Instead, I would suggest to make a tag in the > > commit message mandatory. That way, you could list the associated > > commits using git log --grep=api-breaking > > > > On Tue, Jan 27, 2015 at 1:50 PM, Aljoscha Krettek <aljos...@apache.org> > > wrote: > > > But thats very long, and together with the issue tag I almost always > > > have I lose a lot of my precious 80 characters. > > > > > > On Tue, Jan 27, 2015 at 1:17 PM, Fabian Hueske <fhue...@gmail.com> > > wrote: > > >> I know I argued against "enforcing" commit tags, but how about we make > > two > > >> tags mandatory, i.e., [api-breaking] and [api-extending]? > > >> > > >> 2015-01-07 18:15 GMT+01:00 Gyula Fóra <gyf...@apache.org>: > > >> > > >>> +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 > > >>> > >> > > > > > > > > > > > > >>> > >> > > > > > > > > > > > >>> > >> > > > > > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > >>> > >> > > >>> > > > >>> > > >
