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 <[email protected]> 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 <[email protected]>
>
> beside of the Signed-off-by tag the following are well known:
> Tested-by: Tester <[email protected]>
> Reviewed-by: Reviewer <[email protected]>
> Suggested-by: Idea Creator <[email protected]>
> Acked-by: Component Maintainer <[email protected]>
>
>
> 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
>
