New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add co-author trailers to merge commit. #392

Merged
merged 1 commit into from Apr 8, 2018

Conversation

Projects
None yet
2 participants
@couchand
Contributor

couchand commented Apr 5, 2018

When creating the merge commit for a batch, add a "Co-authored-by" trailer to the commit message for each author contributing to the batch.

Fixes: #382

@notriddle How do you usually test things like this against GitHub? I haven't actually validated any of the code in lib/github/github/server.ex.

https://help.github.com/articles/creating-a-commit-with-multiple-authors/#creating-co-authored-commits-on-the-command-line

@notriddle

This comment has been minimized.

Member

notriddle commented Apr 5, 2018

I have a second instance of bors on a free heroku Dyno, and test against GitHub itself manually. Not great, but doing a full integration test with GitHub would require a bunch of work.

@couchand couchand force-pushed the couchand:feature/co-authors branch from 52c665b to 85b5ffe Apr 5, 2018

@couchand

This comment has been minimized.

Contributor

couchand commented Apr 5, 2018

Fair enough, let me know if there's anything I can do to assist there.

Otherwise, welcome any feedback you have about these proposed changes. Hoping the build passes this time (for some reason I'm having trouble running dogma locally).

bors bot added a commit that referenced this pull request Apr 7, 2018

Merge #393
393: Sort patches before creating merge commit r=notriddle a=couchand

Previously, only the commit message was sorted, but not the
commit parents.  This moves the sort earlier, so that all
processing on the batch happens in the same sorted order.

Fixes: #391 

I removed the previous test that the commit message generation sorted the PRs, since it's no longer valid.  I'd like to add a test that the sorting is still being done, but I'm not really sure where to put it.  The tests in `test/batcher/batcher_test.ex` don't seem to have a clear way to assert on the generated commit.  Perhaps some extensions to `lib/github/github/server_mock.ex` would help?  (This also affects my changes in #392 fwiw...)
@notriddle

Looks like it works well:

bors-ng-test-org/test-repo-2@a6948b2

Rebase it, and then I'll r+ it.

@couchand couchand force-pushed the couchand:feature/co-authors branch from 85b5ffe to 8d9d8ac Apr 8, 2018

@couchand

This comment has been minimized.

Contributor

couchand commented Apr 8, 2018

Great! I rebased and added a bit of validation using the new commit storage scheme in the server mock.

Add co-author trailers to merge commit.
When creating the merge commit for a batch, add a "Co-authored-by"
trailer to the commit message for each author contributing to the batch.

Fixes: #382

@couchand couchand force-pushed the couchand:feature/co-authors branch from 8d9d8ac to 4b7aedb Apr 8, 2018

@notriddle

bors r+

bors bot added a commit that referenced this pull request Apr 8, 2018

Merge #392
392: Add co-author trailers to merge commit. r=notriddle a=couchand

When creating the merge commit for a batch, add a "Co-authored-by" trailer to the commit message for each author contributing to the batch.

Fixes: #382

@notriddle How do you usually test things like this against GitHub?  I haven't actually validated any of the code in `lib/github/github/server.ex`.
@bors

This comment has been minimized.

Contributor

bors bot commented Apr 8, 2018

@bors bors bot merged commit 4b7aedb into bors-ng:master Apr 8, 2018

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment