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

Sort patches before creating merge commit #393

Merged
merged 2 commits into from Apr 7, 2018

Conversation

Projects
None yet
2 participants
@couchand
Contributor

couchand commented Apr 6, 2018

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...)

couchand added some commits Apr 6, 2018

Save synthesized commits in server_mock
This updates the GitHub server mock to save the synthesized commits,
to enable tests to assert on the particular commit messages and
parents of the commits created.
Sort patches before creating merge commit
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

@couchand couchand force-pushed the couchand:fix/pr-sort branch from c6ddf20 to a13735e Apr 6, 2018

@couchand

This comment has been minimized.

Contributor

couchand commented Apr 6, 2018

After spending a few minutes thinking about it, I decided it wouldn't be that hard to just go ahead and update the server mock to save the synthesized commits. Let me know what you think.

@notriddle

bors r+

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...)
@bors

This comment has been minimized.

Contributor

bors bot commented Apr 7, 2018

@bors bors bot merged commit a13735e into bors-ng:master Apr 7, 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