Question about Commit Policy

[Fabian Hueske-2]

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 <[hidden email]>:

> 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 <[hidden email]>
> 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 <[hidden email]>
> > 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 <[hidden email]>
> > 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 <[hidden email]>:
> > >>
> > >>> +1
> > >>> This was much needed :)
> > >>> 2015.01.07. 18:10 ezt írta ("Max Michels" <[hidden email]>):
> > >>>
> > >>> > Nice.
> > >>> >
> > >>> >
> > >>> > On Wed, Jan 7, 2015 at 5:51 PM, Ufuk Celebi <[hidden email]>
> wrote:
> > >>> > > +1
> > >>> > >
> > >>> > > @Stephan: thanks! :-)
> > >>> > >
> > >>> > > On Wed, Jan 7, 2015 at 4:44 PM, Stephan Ewen <[hidden email]>
> > 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 <
> > [hidden email]>
> > >>> > >> 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 <
> > [hidden email]>
> > >>> > >> 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 <[hidden email]>:
> > >>> > >> > >
> > >>> > >> > > > 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 <
> > >>> > >> > [hidden email]
> > >>> > >> > > >
> > >>> > >> > > > 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 <
> > >>> [hidden email]>
> > >>> > >> > 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 <
> > >>> > >> > > > [hidden email]
> > >>> > >> > > > > >
> > >>> > >> > > > > > wrote:
> > >>> > >> > > > > >
> > >>> > >> > > > > > > +1 for the guide, thanks for clarifying the issue
> > >>> > >> > > > > > >
> > >>> > >> > > > > > > On Wed, Jan 7, 2015 at 2:30 PM, Till Rohrmann <
> > >>> > >> > > [hidden email]>
> > >>> > >> > > > > > > wrote:
> > >>> > >> > > > > > >
> > >>> > >> > > > > > > > +1
> > >>> > >> > > > > > > >
> > >>> > >> > > > > > > > On Wed, Jan 7, 2015 at 12:41 PM, Aljoscha Krettek
> <
> > >>> > >> > > > > [hidden email]
> > >>> > >> > > > > > >
> > >>> > >> > > > > > > > wrote:
> > >>> > >> > > > > > > >
> > >>> > >> > > > > > > > > Yes, we should have a guide like that somewhere.
> > >>> > >> > > > > > > > >
> > >>> > >> > > > > > > > >
> > >>> > >> > > > > > > > > On Wed, Jan 7, 2015 at 12:33 PM, Stephan Ewen <
> > >>> > >> > > [hidden email]>
> > >>> > >> > > > > > > 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 <
> > >>> > >> > > > > > > [hidden email]
> > >>> > >> > > > > > > > >
> > >>> > >> > > > > > > > > > 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
> > >>> > >> > > > > > > > > > >
> > >>> > >> > > > > > > > > >
> > >>> > >> > > > > > > > >
> > >>> > >> > > > > > > >
> > >>> > >> > > > > > >
> > >>> > >> > > > > >
> > >>> > >> > > > >
> > >>> > >> > > >
> > >>> > >> > >
> > >>> > >> >
> > >>> > >>
> > >>> >
> > >>>
> >
>

[Henry Saputra]

Just found out about this, thanks Stephan =)

- Henry

On Wed, Jan 7, 2015 at 7:44 AM, Stephan Ewen <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:
>> >
>> > > 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 <
>> [hidden email]
>> > >
>> > > 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 <[hidden email]>
>> 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 <
>> > > [hidden email]
>> > > > >
>> > > > > wrote:
>> > > > >
>> > > > > > +1 for the guide, thanks for clarifying the issue
>> > > > > >
>> > > > > > On Wed, Jan 7, 2015 at 2:30 PM, Till Rohrmann <
>> > [hidden email]>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > +1
>> > > > > > >
>> > > > > > > On Wed, Jan 7, 2015 at 12:41 PM, Aljoscha Krettek <
>> > > > [hidden email]
>> > > > > >
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Yes, we should have a guide like that somewhere.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > On Wed, Jan 7, 2015 at 12:33 PM, Stephan Ewen <
>> > [hidden email]>
>> > > > > > 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 <
>> > > > > > [hidden email]
>> > > > > > > >
>> > > > > > > > > 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
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>

[Max Michels]

Let me clarify my suggestion: Let's put mandatory tags in the second
line of the commit message. That way, they can be filtered using git
log --grep=TAG and do not take away the first line's 80 characters.

On Wed, Jan 28, 2015 at 3:37 AM, Henry Saputra <[hidden email]> wrote:

> Just found out about this, thanks Stephan =)
>
> - Henry
>
> On Wed, Jan 7, 2015 at 7:44 AM, Stephan Ewen <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:
>>> >
>>> > > 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 <
>>> [hidden email]
>>> > >
>>> > > 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 <[hidden email]>
>>> 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 <
>>> > > [hidden email]
>>> > > > >
>>> > > > > wrote:
>>> > > > >
>>> > > > > > +1 for the guide, thanks for clarifying the issue
>>> > > > > >
>>> > > > > > On Wed, Jan 7, 2015 at 2:30 PM, Till Rohrmann <
>>> > [hidden email]>
>>> > > > > > wrote:
>>> > > > > >
>>> > > > > > > +1
>>> > > > > > >
>>> > > > > > > On Wed, Jan 7, 2015 at 12:41 PM, Aljoscha Krettek <
>>> > > > [hidden email]
>>> > > > > >
>>> > > > > > > wrote:
>>> > > > > > >
>>> > > > > > > > Yes, we should have a guide like that somewhere.
>>> > > > > > > >
>>> > > > > > > >
>>> > > > > > > > On Wed, Jan 7, 2015 at 12:33 PM, Stephan Ewen <
>>> > [hidden email]>
>>> > > > > > 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 <
>>> > > > > > [hidden email]
>>> > > > > > > >
>>> > > > > > > > > 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
>>> > > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>