Question about Commit Policy
Locked
12
12
Question about Commit Policy
 
|
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 |
Re: Question about Commit Policy
|
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 > |
Re: Question about Commit Policy
|
|
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 > > > |
Re: Question about Commit Policy
|
+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 > > > > > > |
Re: Question about Commit Policy
|
|
+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 > > > > > > > > > > |
|
|
+1
Perhaps declaring the component should be optional because it leaves less space for the actual commit message. We should definitely add this guide to the wiki. 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 >> > > > >> > > >> > >> |
Re: Question about Commit Policy
|
|
In reply to this post by Márton Balassi
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 > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
I would only start a vote if somebody objects.
How about adding this rule to our website, to make it even more official. I would like to establish a document that contains all the rules we agreed on. Similarly to the coding guidelines ( http://flink.apache.org/coding_guidelines.html) we could establish Community guidelines? 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 > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
In reply to this post by Stephan Ewen
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 > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
+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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
+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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
Re: Question about Commit Policy
|
|
+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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
|
|
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 >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> |
Re: Question about Commit Policy
|
|
+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 > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > |
Re: Question about Commit Policy
|
|
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 > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > |
Re: Question about Commit Policy
|
|
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 >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > >> > > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > >> >> > >> |
|
|
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 >>> > >> > > > > > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > >>> > >> > > >>> > >> > >>> > >> >>> > >>> |
Re: Question about Commit Policy
|
|
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 > >>> > >> > > > > > > > > > > > >>> > >> > > > > > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > >>> > >> > > >>> > >> > >>> > > >>> > |
«
Return to (DEPRECATED) Apache Flink Mailing List archive.
|
1 view|%1 views
| Free forum by Nabble | Edit this page |