You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Roger Meier <ro...@bufferoverflow.ch> on 2014/04/05 02:34:22 UTC
[DISCUSS] contribution and commit messages
All,
We had several discussion about commit messages on multiple channels.
Let's discuss this here on the dev list.
Question 1: Documentation
merge HowToCommit + HowToContribute into /CONTRIBUTE.md
and include it easily with our brand new Apache CMS on the web site?
Question 2: Developer's Certificate of Origin
Today we have this commit style:
--- http://thrift.apache.org/docs/committers/HowToCommit ---
THRIFT-###:<Jira description>
Client: <component>
Patch: <Name of person contributing the patch>
Description of what was fixed or addressed.
<%
if this is a github pull request then copy the below block
from the GitHub email that came to dev@ list, this will
automatically close the GitHub pull request
%>
Github Pull Request: This closes #XX
--- http://thrift.apache.org/docs/committers/HowToCommit ---
Hmm, I did something wrong on the Lua commit...sorry!
What do you think about git best practice known from other projects to
get *Developer's Certificate of Origin*, such as here:
- https://www.kernel.org/doc/Documentation/SubmittingPatches
- http://www.denx.de/wiki/U-Boot/Patches
e.g. *git commit -s* automatically adds a Signed-off-by line to the
commit message.
Signed-off-by: Patch Creator <pa...@example.org>
beside of the Signed-off-by tag the following are well known:
Tested-by: Tester <te...@example.org>
Reviewed-by: Reviewer <re...@example.org>
Suggested-by: Idea Creator <id...@example.org>
Acked-by: Component Maintainer <ma...@example.org>
Question 3: git am
Would it make sense to start using this?
Question 4: patch creation
What about that workflow:
git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
cd thrift
git pull origin master
# create a local feature branch
git checkout -b THRIFT-1681
# make the changes and commit to your local feature branch
git commit -a
# format patch
git format-patch -1
# submit the patch or create a pull request
all the best!
roger
;-r
Re: [DISCUSS] contribution and commit messages
Posted by Jens Geyer <je...@hotmail.com>.
> god patches
Of course I meant "good patches". Funny typo anyway ;-)
-----Ursprüngliche Nachricht-----
From: Jens Geyer
Sent: Sunday, April 6, 2014 1:15 PM
To: dev@thrift.apache.org
Subject: Re: [DISCUSS] contribution and commit messages
Hi Roger,
> What do you think about Q1,Q2 and Q4?
Basically I can live with any policy, as long as we have one, and as long as
it doesn't make things only more complicated to handle. Every "more
complicated" is usually directly related to "more prone to error" and "takes
more time".
For example, the ability to accept pull requests is what I consider a big
improvement: Getting used to the new format of a slightly changed commit
message absolutey pays off compared to the bunch of hurdles that have been
removed, allowing us to accept god patches more quickly. Thanks again to
Jake for making this possible.
Have fun,
JensG
-----Ursprüngliche Nachricht-----
From: Roger Meier
Sent: Saturday, April 5, 2014 6:13 PM
To: dev@thrift.apache.org
Subject: Re: [DISCUSS] contribution and commit messages
Jens,
It was not my intend to bring in such a strong policy as known from the
Linux Kernel. Fully agree, this is too heavy for Apache Thrift.
Each patch need a JIRA ticket, no mailing list only patches as with
Linux Kernel.
git am is especially useful to apply a patch created by *git
format-patch -1* or a github pull request. both formats can be used
directly and git am keeps the original commit message, author, etc.
What do you think about Q1,Q2 and Q4?
regards
-roger
On 05.04.2014 13:28, Jens Geyer wrote:
> Hi all,
>
>> Question 3: git am
>> git format-patch -1
>> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
> Cited from the linked doc:
>
>> 7) No MIME, no links, no compression, no attachments. Just plain text.
>>
>> [...], all patches should be submitting e-mail "inline".
>> WARNING: Be wary of your editor's word-wrap corrupting your patch,
>> if you choose to cut-n-paste your patch.
>
> I have a bad feeeling regarding that specific part. We already
> experience and waste time fighting strange problems from time to time,
> caused by incorrect EOL styles, and whitespace changes. Sending a patch
> as plain text mail sounds very much like adding a new problem source to
> the game. It probably works for Linux only projects, but Thrift is the
> exact opposite of $PLATFORM only, and we have to be careful with such
> things. However, getting a patch as file attachment is probably ok, as
> long as we have a reference to JIRA for everything. How can the latter
> be ensured?
>
> JensG
>
>
> -----Ursprüngliche Nachricht----- From: Roger Meier
> Sent: Saturday, April 5, 2014 2:34 AM
> To: dev@thrift.apache.org
> Subject: [DISCUSS] contribution and commit messages
>
> All,
>
> We had several discussion about commit messages on multiple channels.
> Let's discuss this here on the dev list.
>
> Question 1: Documentation
> merge HowToCommit + HowToContribute into /CONTRIBUTE.md
> and include it easily with our brand new Apache CMS on the web site?
>
> Question 2: Developer's Certificate of Origin
> Today we have this commit style:
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> THRIFT-###:<Jira description>
> Client: <component>
> Patch: <Name of person contributing the patch>
>
> Description of what was fixed or addressed.
>
> <%
> if this is a github pull request then copy the below block
> from the GitHub email that came to dev@ list, this will
> automatically close the GitHub pull request
> %>
> Github Pull Request: This closes #XX
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> Hmm, I did something wrong on the Lua commit...sorry!
>
> What do you think about git best practice known from other projects to
> get *Developer's Certificate of Origin*, such as here:
> - https://www.kernel.org/doc/Documentation/SubmittingPatches
> - http://www.denx.de/wiki/U-Boot/Patches
>
> e.g. *git commit -s* automatically adds a Signed-off-by line to the
> commit message.
> Signed-off-by: Patch Creator <pa...@example.org>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <te...@example.org>
> Reviewed-by: Reviewer <re...@example.org>
> Suggested-by: Idea Creator <id...@example.org>
> Acked-by: Component Maintainer <ma...@example.org>
>
>
> Question 3: git am
> Would it make sense to start using this?
>
> Question 4: patch creation
> What about that workflow:
> git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
> cd thrift
> git pull origin master
> # create a local feature branch
> git checkout -b THRIFT-1681
> # make the changes and commit to your local feature branch
> git commit -a
> # format patch
> git format-patch -1
> # submit the patch or create a pull request
>
>
> all the best!
> roger
> ;-r
>
Re: [DISCUSS] contribution and commit messages
Posted by Jens Geyer <je...@hotmail.com>.
Hi Roger,
> What do you think about Q1,Q2 and Q4?
Basically I can live with any policy, as long as we have one, and as long as
it doesn't make things only more complicated to handle. Every "more
complicated" is usually directly related to "more prone to error" and "takes
more time".
For example, the ability to accept pull requests is what I consider a big
improvement: Getting used to the new format of a slightly changed commit
message absolutey pays off compared to the bunch of hurdles that have been
removed, allowing us to accept god patches more quickly. Thanks again to
Jake for making this possible.
Have fun,
JensG
-----Ursprüngliche Nachricht-----
From: Roger Meier
Sent: Saturday, April 5, 2014 6:13 PM
To: dev@thrift.apache.org
Subject: Re: [DISCUSS] contribution and commit messages
Jens,
It was not my intend to bring in such a strong policy as known from the
Linux Kernel. Fully agree, this is too heavy for Apache Thrift.
Each patch need a JIRA ticket, no mailing list only patches as with
Linux Kernel.
git am is especially useful to apply a patch created by *git
format-patch -1* or a github pull request. both formats can be used
directly and git am keeps the original commit message, author, etc.
What do you think about Q1,Q2 and Q4?
regards
-roger
On 05.04.2014 13:28, Jens Geyer wrote:
> Hi all,
>
>> Question 3: git am
>> git format-patch -1
>> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
> Cited from the linked doc:
>
>> 7) No MIME, no links, no compression, no attachments. Just plain text.
>>
>> [...], all patches should be submitting e-mail "inline".
>> WARNING: Be wary of your editor's word-wrap corrupting your patch,
>> if you choose to cut-n-paste your patch.
>
> I have a bad feeeling regarding that specific part. We already
> experience and waste time fighting strange problems from time to time,
> caused by incorrect EOL styles, and whitespace changes. Sending a patch
> as plain text mail sounds very much like adding a new problem source to
> the game. It probably works for Linux only projects, but Thrift is the
> exact opposite of $PLATFORM only, and we have to be careful with such
> things. However, getting a patch as file attachment is probably ok, as
> long as we have a reference to JIRA for everything. How can the latter
> be ensured?
>
> JensG
>
>
> -----Ursprüngliche Nachricht----- From: Roger Meier
> Sent: Saturday, April 5, 2014 2:34 AM
> To: dev@thrift.apache.org
> Subject: [DISCUSS] contribution and commit messages
>
> All,
>
> We had several discussion about commit messages on multiple channels.
> Let's discuss this here on the dev list.
>
> Question 1: Documentation
> merge HowToCommit + HowToContribute into /CONTRIBUTE.md
> and include it easily with our brand new Apache CMS on the web site?
>
> Question 2: Developer's Certificate of Origin
> Today we have this commit style:
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> THRIFT-###:<Jira description>
> Client: <component>
> Patch: <Name of person contributing the patch>
>
> Description of what was fixed or addressed.
>
> <%
> if this is a github pull request then copy the below block
> from the GitHub email that came to dev@ list, this will
> automatically close the GitHub pull request
> %>
> Github Pull Request: This closes #XX
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> Hmm, I did something wrong on the Lua commit...sorry!
>
> What do you think about git best practice known from other projects to
> get *Developer's Certificate of Origin*, such as here:
> - https://www.kernel.org/doc/Documentation/SubmittingPatches
> - http://www.denx.de/wiki/U-Boot/Patches
>
> e.g. *git commit -s* automatically adds a Signed-off-by line to the
> commit message.
> Signed-off-by: Patch Creator <pa...@example.org>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <te...@example.org>
> Reviewed-by: Reviewer <re...@example.org>
> Suggested-by: Idea Creator <id...@example.org>
> Acked-by: Component Maintainer <ma...@example.org>
>
>
> Question 3: git am
> Would it make sense to start using this?
>
> Question 4: patch creation
> What about that workflow:
> git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
> cd thrift
> git pull origin master
> # create a local feature branch
> git checkout -b THRIFT-1681
> # make the changes and commit to your local feature branch
> git commit -a
> # format patch
> git format-patch -1
> # submit the patch or create a pull request
>
>
> all the best!
> roger
> ;-r
>
Re: [DISCUSS] contribution and commit messages
Posted by Roger Meier <ro...@bufferoverflow.ch>.
Jens,
It was not my intend to bring in such a strong policy as known from the
Linux Kernel. Fully agree, this is too heavy for Apache Thrift.
Each patch need a JIRA ticket, no mailing list only patches as with
Linux Kernel.
git am is especially useful to apply a patch created by *git
format-patch -1* or a github pull request. both formats can be used
directly and git am keeps the original commit message, author, etc.
What do you think about Q1,Q2 and Q4?
regards
-roger
On 05.04.2014 13:28, Jens Geyer wrote:
> Hi all,
>
>> Question 3: git am
>> git format-patch -1
>> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
> Cited from the linked doc:
>
>> 7) No MIME, no links, no compression, no attachments. Just plain text.
>>
>> [...], all patches should be submitting e-mail "inline".
>> WARNING: Be wary of your editor's word-wrap corrupting your patch,
>> if you choose to cut-n-paste your patch.
>
> I have a bad feeeling regarding that specific part. We already
> experience and waste time fighting strange problems from time to time,
> caused by incorrect EOL styles, and whitespace changes. Sending a patch
> as plain text mail sounds very much like adding a new problem source to
> the game. It probably works for Linux only projects, but Thrift is the
> exact opposite of $PLATFORM only, and we have to be careful with such
> things. However, getting a patch as file attachment is probably ok, as
> long as we have a reference to JIRA for everything. How can the latter
> be ensured?
>
> JensG
>
>
> -----Ursprüngliche Nachricht----- From: Roger Meier
> Sent: Saturday, April 5, 2014 2:34 AM
> To: dev@thrift.apache.org
> Subject: [DISCUSS] contribution and commit messages
>
> All,
>
> We had several discussion about commit messages on multiple channels.
> Let's discuss this here on the dev list.
>
> Question 1: Documentation
> merge HowToCommit + HowToContribute into /CONTRIBUTE.md
> and include it easily with our brand new Apache CMS on the web site?
>
> Question 2: Developer's Certificate of Origin
> Today we have this commit style:
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> THRIFT-###:<Jira description>
> Client: <component>
> Patch: <Name of person contributing the patch>
>
> Description of what was fixed or addressed.
>
> <%
> if this is a github pull request then copy the below block
> from the GitHub email that came to dev@ list, this will
> automatically close the GitHub pull request
> %>
> Github Pull Request: This closes #XX
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> Hmm, I did something wrong on the Lua commit...sorry!
>
> What do you think about git best practice known from other projects to
> get *Developer's Certificate of Origin*, such as here:
> - https://www.kernel.org/doc/Documentation/SubmittingPatches
> - http://www.denx.de/wiki/U-Boot/Patches
>
> e.g. *git commit -s* automatically adds a Signed-off-by line to the
> commit message.
> Signed-off-by: Patch Creator <pa...@example.org>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <te...@example.org>
> Reviewed-by: Reviewer <re...@example.org>
> Suggested-by: Idea Creator <id...@example.org>
> Acked-by: Component Maintainer <ma...@example.org>
>
>
> Question 3: git am
> Would it make sense to start using this?
>
> Question 4: patch creation
> What about that workflow:
> git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
> cd thrift
> git pull origin master
> # create a local feature branch
> git checkout -b THRIFT-1681
> # make the changes and commit to your local feature branch
> git commit -a
> # format patch
> git format-patch -1
> # submit the patch or create a pull request
>
>
> all the best!
> roger
> ;-r
>
Re: [DISCUSS] contribution and commit messages
Posted by Chris Bannister <c....@gmail.com>.
To get around the line ending issues you could enforce it with
.gitattributes. Im not sure what git format-patch does with line endings
On 5 April 2014 12:28, Jens Geyer <je...@hotmail.com> wrote:
> Hi all,
>
> Question 3: git am
>> git format-patch -1
>> https://www.kernel.org/doc/Documentation/SubmittingPatches
>>
>
> Cited from the linked doc:
>
> 7) No MIME, no links, no compression, no attachments. Just plain text.
>>
>> [...], all patches should be submitting e-mail "inline".
>> WARNING: Be wary of your editor's word-wrap corrupting your patch,
>> if you choose to cut-n-paste your patch.
>>
>
> I have a bad feeeling regarding that specific part. We already experience
> and waste time fighting strange problems from time to time, caused by
> incorrect EOL styles, and whitespace changes. Sending a patch as plain text
> mail sounds very much like adding a new problem source to the game. It
> probably works for Linux only projects, but Thrift is the exact opposite of
> $PLATFORM only, and we have to be careful with such things. However,
> getting a patch as file attachment is probably ok, as long as we have a
> reference to JIRA for everything. How can the latter be ensured?
>
> JensG
>
>
> -----Ursprüngliche Nachricht----- From: Roger Meier
> Sent: Saturday, April 5, 2014 2:34 AM
> To: dev@thrift.apache.org
> Subject: [DISCUSS] contribution and commit messages
>
>
> All,
>
> We had several discussion about commit messages on multiple channels.
> Let's discuss this here on the dev list.
>
> Question 1: Documentation
> merge HowToCommit + HowToContribute into /CONTRIBUTE.md
> and include it easily with our brand new Apache CMS on the web site?
>
> Question 2: Developer's Certificate of Origin
> Today we have this commit style:
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> THRIFT-###:<Jira description>
> Client: <component>
> Patch: <Name of person contributing the patch>
>
> Description of what was fixed or addressed.
>
> <%
> if this is a github pull request then copy the below block
> from the GitHub email that came to dev@ list, this will
> automatically close the GitHub pull request
> %>
> Github Pull Request: This closes #XX
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> Hmm, I did something wrong on the Lua commit...sorry!
>
> What do you think about git best practice known from other projects to
> get *Developer's Certificate of Origin*, such as here:
> - https://www.kernel.org/doc/Documentation/SubmittingPatches
> - http://www.denx.de/wiki/U-Boot/Patches
>
> e.g. *git commit -s* automatically adds a Signed-off-by line to the
> commit message.
> Signed-off-by: Patch Creator <pa...@example.org>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <te...@example.org>
> Reviewed-by: Reviewer <re...@example.org>
> Suggested-by: Idea Creator <id...@example.org>
> Acked-by: Component Maintainer <ma...@example.org>
>
>
> Question 3: git am
> Would it make sense to start using this?
>
> Question 4: patch creation
> What about that workflow:
> git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
> cd thrift
> git pull origin master
> # create a local feature branch
> git checkout -b THRIFT-1681
> # make the changes and commit to your local feature branch
> git commit -a
> # format patch
> git format-patch -1
> # submit the patch or create a pull request
>
>
> all the best!
> roger
> ;-r
>
Re: [DISCUSS] contribution and commit messages
Posted by Jens Geyer <je...@hotmail.com>.
Hi all,
> Question 3: git am
> git format-patch -1
> https://www.kernel.org/doc/Documentation/SubmittingPatches
Cited from the linked doc:
> 7) No MIME, no links, no compression, no attachments. Just plain text.
>
> [...], all patches should be submitting e-mail "inline".
> WARNING: Be wary of your editor's word-wrap corrupting your patch,
> if you choose to cut-n-paste your patch.
I have a bad feeeling regarding that specific part. We already experience
and waste time fighting strange problems from time to time, caused by
incorrect EOL styles, and whitespace changes. Sending a patch as plain text
mail sounds very much like adding a new problem source to the game. It
probably works for Linux only projects, but Thrift is the exact opposite of
$PLATFORM only, and we have to be careful with such things. However, getting
a patch as file attachment is probably ok, as long as we have a reference to
JIRA for everything. How can the latter be ensured?
JensG
-----Ursprüngliche Nachricht-----
From: Roger Meier
Sent: Saturday, April 5, 2014 2:34 AM
To: dev@thrift.apache.org
Subject: [DISCUSS] contribution and commit messages
All,
We had several discussion about commit messages on multiple channels.
Let's discuss this here on the dev list.
Question 1: Documentation
merge HowToCommit + HowToContribute into /CONTRIBUTE.md
and include it easily with our brand new Apache CMS on the web site?
Question 2: Developer's Certificate of Origin
Today we have this commit style:
--- http://thrift.apache.org/docs/committers/HowToCommit ---
THRIFT-###:<Jira description>
Client: <component>
Patch: <Name of person contributing the patch>
Description of what was fixed or addressed.
<%
if this is a github pull request then copy the below block
from the GitHub email that came to dev@ list, this will
automatically close the GitHub pull request
%>
Github Pull Request: This closes #XX
--- http://thrift.apache.org/docs/committers/HowToCommit ---
Hmm, I did something wrong on the Lua commit...sorry!
What do you think about git best practice known from other projects to
get *Developer's Certificate of Origin*, such as here:
- https://www.kernel.org/doc/Documentation/SubmittingPatches
- http://www.denx.de/wiki/U-Boot/Patches
e.g. *git commit -s* automatically adds a Signed-off-by line to the
commit message.
Signed-off-by: Patch Creator <pa...@example.org>
beside of the Signed-off-by tag the following are well known:
Tested-by: Tester <te...@example.org>
Reviewed-by: Reviewer <re...@example.org>
Suggested-by: Idea Creator <id...@example.org>
Acked-by: Component Maintainer <ma...@example.org>
Question 3: git am
Would it make sense to start using this?
Question 4: patch creation
What about that workflow:
git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
cd thrift
git pull origin master
# create a local feature branch
git checkout -b THRIFT-1681
# make the changes and commit to your local feature branch
git commit -a
# format patch
git format-patch -1
# submit the patch or create a pull request
all the best!
roger
;-r
Re: [DISCUSS] contribution and commit messages
Posted by Jake Farrell <jf...@apache.org>.
Hey Roger
I'm in favor of keeping things as simple for the committer as possibly,
introducing more steps or flags just leave the possibility for something to
get missed.
1. Done, see doc/contributing.md and doc/committers.md
2. The author field is what we currently use for who has {tested, reviewed,
committed} a given patch, I do not think we need to introduce more flags
that could be forgotten when submitting a contribution. Along those lines,
I think that what your had as the commit message for the lua contribution
was exactly what we need (having the client in commit messages is redundant
as our changelog is made up from the jira tickets and those *should* all
include a component and fix version)
3. Patches are not included with the github pull request email
notifications and it is not planned to add them. (We (infra hat on now) had
discussions when working on the pull request notifications and decided to
not include them intentionally).
4. format-patch is the compliment to git-am, using the unified diff should
be fine for our needs. git show HEAD > thrift-xxx.patch matches what github
is giving us with the <pull request id>.patch.
-Jake
On Fri, Apr 4, 2014 at 8:34 PM, Roger Meier <ro...@bufferoverflow.ch> wrote:
> All,
>
> We had several discussion about commit messages on multiple channels.
> Let's discuss this here on the dev list.
>
> Question 1: Documentation
> merge HowToCommit + HowToContribute into /CONTRIBUTE.md
> and include it easily with our brand new Apache CMS on the web site?
>
> Question 2: Developer's Certificate of Origin
> Today we have this commit style:
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> THRIFT-###:<Jira description>
> Client: <component>
> Patch: <Name of person contributing the patch>
>
> Description of what was fixed or addressed.
>
> <%
> if this is a github pull request then copy the below block
> from the GitHub email that came to dev@ list, this will
> automatically close the GitHub pull request
> %>
> Github Pull Request: This closes #XX
> --- http://thrift.apache.org/docs/committers/HowToCommit ---
> Hmm, I did something wrong on the Lua commit...sorry!
>
> What do you think about git best practice known from other projects to get
> *Developer's Certificate of Origin*, such as here:
> - https://www.kernel.org/doc/Documentation/SubmittingPatches
> - http://www.denx.de/wiki/U-Boot/Patches
>
> e.g. *git commit -s* automatically adds a Signed-off-by line to the commit
> message.
> Signed-off-by: Patch Creator <pa...@example.org>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <te...@example.org>
> Reviewed-by: Reviewer <re...@example.org>
> Suggested-by: Idea Creator <id...@example.org>
> Acked-by: Component Maintainer <ma...@example.org>
>
>
> Question 3: git am
> Would it make sense to start using this?
>
> Question 4: patch creation
> What about that workflow:
> git clone https://git-wip-us.apache.org/repos/asf/thrift.git thrift
> cd thrift
> git pull origin master
> # create a local feature branch
> git checkout -b THRIFT-1681
> # make the changes and commit to your local feature branch
> git commit -a
> # format patch
> git format-patch -1
> # submit the patch or create a pull request
>
>
> all the best!
> roger
> ;-r
>