Mailing-List: contact dev-help@flink.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@flink.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CANC1h_u5Zvd5CP+mrs7yeo0E_EQdoMPSPkM0CBAPoun4_P00yw@mail.gmail.com>
References: 
 <CANMXwW0g8VMZTO-er_2YZeG7FHPy0CZYDC1VAVoUaS4bH-50nQ@mail.gmail.com>
 <CANC1h_ueLfzANCCMTbc=8LG7K6KgQTKsy8WRrGZarkh0Z7qh0g@mail.gmail.com>
 <CANMXwW0o=DEKOUT--LGY96FDXerS2in_rWc8jhP3gg++jPSAbw@mail.gmail.com>
 <CAC27z=PYCMvK2V-O-ffEYzOcg0fqJ4mH-TR5P4CYVbK4+Rxr3g@mail.gmail.com>
 <CAKADb_PgsQHhQfwaCtQnkdmWdxgASE6_avCjYiLDW3jzJU9LxQ@mail.gmail.com>
 <CANC1h_t5QGmjsB6FyewS36d8GuBZWVQUt2qm8MK3x9+GLk-_Sw@mail.gmail.com>
 <CAKADb_M7HPnWc_ObsqaePfCfX_f4xOaL7=9zQ-LQFCgL+_3yMA@mail.gmail.com>
 <CANC1h_tdLOsgnC9-8zng3FzxCQVyOCNO5BOkXRi+AbR6yyipGQ@mail.gmail.com>
 <CAAdrtT10axjaNgp5auyGUvSXtiFngS83NKAC4u4W1p1_k5qC-A@mail.gmail.com>
 <CAGWx-_sgnsRN8L1TjEa87yu4q098xgrWL0r3D6wxOzRNy9Ly6Q@mail.gmail.com>
 <CANC1h_u5Zvd5CP+mrs7yeo0E_EQdoMPSPkM0CBAPoun4_P00yw@mail.gmail.com>
From: Ufuk Celebi <uce@apache.org>
Date: Wed, 7 Jan 2015 17:51:50 +0100
Message-ID: 
 <CAKiyyaHehafykrD+nSM8P0n6P8-N1WtS8nQ=qPb20LSVUdjthQ@mail.gmail.com>
Subject: Re: Question about Commit Policy
To: Flink dev list <dev@flink.incubator.apache.org>
Content-Type: multipart/alternative; boundary=001a11c344b4d9e92d050c12bf77

--001a11c344b4d9e92d050c12bf77
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

+1

@Stephan: thanks! :-)

On Wed, Jan 7, 2015 at 4:44 PM, 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+developmen=
t+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 wi=
ll
> > 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, w=
e
> > > 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 compone=
nt
> > 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=C3=A1rton 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 selecti=
on
> > > > change
> > > > > for streaming?" type question I just had to answer would have bee=
n
> 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 A=
PI
> > > > rework
> > > > > for streaming". I do prefer the former one, because it is not onl=
y
> > more
> > > > > straight-forward to understand, but a bit shorter as well.
> > > > > Of course there are counter-examples, like "[streaming] DataStrea=
m
> > > > > 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=C3=A1rton 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 go=
od
> > > point
> > > > > to
> > > > > > do
> > > > > > > > so.
> > > > > > > > > >
> > > > > > > > > > I personally find it good to have changes associated wi=
th
> > 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 / efficienc=
y
> > > > (simple
> > > > > > > > > rewriting
> > > > > > > > > > a loop / condition / method interaction) if no behavior
> is
> > > > > changed
> > > > > > > > > >
> > > > > > > > > > =3D=3D> 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 sca=
la
> > api
> > > > > > > > > > [FLINK-1234] [optimizer] Make optimizer aware that it c=
an
> > > > 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
> > > > > > > > > >
> > > > > > > > > > =3D=3D> Changes that come with a pull request should ha=
ve 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
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

--001a11c344b4d9e92d050c12bf77--