Change log and Github


(Christian Müller) #1

I have found one issue that we need to discuss. The merge commits do not actually have the subject you gave it when opening the pull request but are always something in the lines of "Merge pull request #99 from foo/bar/baz"
this means we have no reliable change log atm
see https://github.com/neos/neos-development-collection/commits/master

Input from @aertmann and @kdambekalns would be great. I guess we need to rethink how we create our change logs and how we go about with the commit messages…


(Christian Müller) #2

We could make the message body of the pull request nice and matching our guidelines and just pull in the body for those merge commits then…


(Karsten Dambekalns) #3

We could do all merges by hand, then we can do whatever we like to the commit subject. Then again, a lot of work. Maybe we should rethink our reasoning behind wanting to use the merge commits. It was to lower the barrier, so that people wouldn’t have to (always) write (absolutely) proper commit messages.

Since a merge only pulls in changes, and the action actually happens in the commits on a branch, this might be the wrong approach… Even for merge conflicts - those should be resolved by merging the target into the feature branch, not while merging the feature into the target branch.

Now, considering the upmerges, those will never be able to inform about all that is merged. There might be one, three or a dozen changes being merged, that won’t fit into a single subject line anyway. That’s why most git tools will just list the sibject lines in the body of a merge commit message.

All that being said, by now I think our best bet is to try to get proper commit messages to begin with (yes, why not let people force push on their first two, three PRs until the get the hang of it?). Then we can create change logs by ignoring merges (or not) as we see fit.

And in the end, most people would probably be totally fine with a simple compare view on GitHub, with all the navigation into the code it provdes. A lot of libraries and projects use nothing more for their change logs.


(Christian Müller) #4

I am mainly talking about PR merges (so bugfixes, features etc.) The upmerges wouldn’t necessarily need a nice message because important are the PR merges again which are pulled into the branch with the upmerge so that would be fine. IMHO the big -1 for requiring proper commit messages are “drive by” changes that will probably not be touched by the original commiter two or three times. I see high value in those as usually they are bugfixes for edge cases and stuff and we won’t be able to convert those people into repeated commiters. And then it becomes difficult. After which time do we take the change over? How does that happen? Who takes care if no one of the team suffered from the bug… As we usually won’t be able to push into the PR branch basically we would have to checkout the branch, fix the thing by amending, push in own fork and create a new PR. Sounds like a lot of work for a commit message and would reduce the original commiter to the author only even though they basically did most of the work. So either we loose valuable contributions because they rot away just like in Gerrit or we need to adopt them (which I also do not see happening).
Then I am actually more inclined to really just add proper descriptions to the merge commits and do not care about the subject line.


(Aske Ertmann) #5

Hah did not expect that, find that auto created conversation comment from the last commit message description serving as pull request body logic a bit weird to be honest.

Anyway it is possible to add the stuff when merging a pull request, see https://github.com/neos/neos-development-collection/commit/65eb31173519430f510622551916f2f45772a1a9

However GH doesn’t copy it by default so you’d have to copy and paste it when merging the pull request.

This workflow would be fine with me, but processes like these often have mistakes. And for everything merged so far we don’t have a good log.

We could easily fix this by getting the message from the GH API when generating the log and use if there is no description in the merge commit.

Example: see the “body” field https://api.github.com/repos/neos/neos-development-collection/pulls/110

This would actually allow us to easily adjust the log without creating it first and then amending it. Keep in mind that the log with this approach is not a commit log, but more a release log made easier. The commit log is available on GH nicely already.

I still think it makes sense not to force commit messages to allow easier contribution.

I’d say that should never happen, a PR should only be merged if it doesn’t conflict so we use GH’s processes for managing PRs. If a PR is dead a new one can be created instead where the conflict is fixed and the normal process is followed.

In short: We can easily make it work the way we decided upon and -1 for changing it from my side.

Generally agree to Christians last commit.


(Aske Ertmann) #6

Additionally I’d like to improve the log a bit with actual links to PRs and commits instead of the plain style we’ve had so far, which supports making the creation of the log a bit smarter than previously. E.g. linking to Jira issues. It’s just a script we only use internally anyway.


(Christian Müller) #7

I am not quite sure how you would like to do it @aertmann?


(Aske Ertmann) #8

Like we initially agreed upon, use the pull request merges for the commit log. And just pull GH when creating the log instead of of relying on the process being followed, because it’s easy to fix afterwards without having to touch the Git repository.

We should just strive for copying the PR body (first commit) into the box when pressing the merge button so it’s also available in the log, but if someone forgets we can still make a nice log.


(Karsten Dambekalns) #9

Of course a PR is only merged if it applies clean. But for this you need to resolve existing conflicts, and that needs to be done my merging the target branch into the PR branch. That merge is then part of the PR and can be reviewed; the PR can then be merged through the GitHub UI.

I guess we mean the same, just wanted to make sure… there is no need to close a PR that has conflicts and open a new PR - on the contrary, that potentially lowers the visibility of any conflicts and their resolution.


(Karsten Dambekalns) #10

Fine with me, either way (whatever ways there might be).

I’m happy to be able to see what has been merged without having to compare disconnected branches, everything else is the proverbial icing on the cake (to me). I still think something like http://git-scm.com/book/ch5-3.html#The-Shortlog would be enough, even. But that relies on proper commit messages again. So… whatever… :slight_smile:


(Karsten Dambekalns) #11

The question is - what is a proper commit message? I’d expect a usable description even in a “drive by change”, I mean, rly? We got rid of commit message footers, a dive-vy commiter would probably not know about an issue ID anyway, and the commit subject prefix, well… who cares?

But if someone just writes “Fix bug” on a relevant bugfix, I’d be surprised (and disappointed!). And I guess it won’t happen often, so in those cases we could just hub am url-of-the-pr; git commit --amend; …

Anyway, I won’t force my belief in the existence of common sense and an urge for quality on you guys. :smile:


(Aske Ertmann) #12

I’d even suggest to rebase instead of merging conflicts in the PR branches, at least I’d do that personally.


(Aske Ertmann) #13

And thanks for the responses. Let’s do it this way. To summarize:

Use pull request title and body (first PR comment that is always available and automatically created) for proper commit message used for the change log. This should be formatted in the way described in Commit Message Style

When merging a pull request, copy body text under the automatically inserted title in the “merge commit message”.

The log fetches the pull request title and body directly from Github, which allows fixing it post merge. However the body should still be copied when merging, since it provides better details in the actual Git log and in the GH commit overview.


(Christian Müller) #14

I see, nice idea, works for me…


(Aske Ertmann) #15

While implementing this I came to the conclusion that it makes sense not to include the base distribution and demo site changes in the changelog and only include what’s in the development collection. Reason being that the base rarely has any mention worthy changes and the demo site not being relevant for most projects. Any objections? /cc @kdambekalns @christianm


(Christian Müller) #16

Sounds good to me. If anything noteworthy in the distribution happens we can still put that in the release notes or something…


(Aske Ertmann) #17

The pull request based log has been implemented and described in Committers Guide


(Aske Ertmann) #18