flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Max Michels <...@data-artisans.com>
Subject Re: Question about Commit Policy
Date Wed, 28 Jan 2015 09:14:35 GMT
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 <henry.saputra@gmail.com> wrote:
> Just found out about this, thanks Stephan =)
>
> - Henry
>
> On Wed, Jan 7, 2015 at 7:44 AM, Stephan Ewen <sewen@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 <ktzoumas@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 <fhueske@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 <sewen@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.marton@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 <sewen@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.marton@gmail.com
>>> > > > >
>>> > > > > wrote:
>>> > > > >
>>> > > > > > +1 for the guide, thanks for clarifying the issue
>>> > > > > >
>>> > > > > > On Wed, Jan 7, 2015 at 2:30 PM, Till Rohrmann <
>>> > trohrmann@apache.org>
>>> > > > > > wrote:
>>> > > > > >
>>> > > > > > > +1
>>> > > > > > >
>>> > > > > > > On Wed, Jan 7, 2015 at 12:41 PM, Aljoscha Krettek
<
>>> > > > aljoscha@apache.org
>>> > > > > >
>>> > > > > > > wrote:
>>> > > > > > >
>>> > > > > > > > Yes, we should have a guide like that somewhere.
>>> > > > > > > >
>>> > > > > > > >
>>> > > > > > > > On Wed, Jan 7, 2015 at 12:33 PM, Stephan Ewen
<
>>> > sewen@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 <
>>> > > > > > aljoscha@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
>>> > > > > > > > > >
>>> > > > > > > > >
>>> > > > > > > >
>>> > > > > > >
>>> > > > > >
>>> > > > >
>>> > > >
>>> > >
>>> >
>>>

Mime
View raw message