Merging Strategy

[ben-e-whitney]

1. Currently we're getting a merge commit for each pull request accepted (f81d648 for f81d648, f81d648 for f81d648, etc.). @qliu21, would you be willing to rebase pull requests instead? Here's some documentation. It seems you can either choose the merge option for each pull request individually or set it globally for the repository.
2. Would it be possible to give someone from ORNL permission to accept pull requests into some common branch? I nominate myself – currently I only have permission to push to unstructured. It we want to keep master protected, one option would be to set up a development branch and periodically merge it into master. Let me know what you think. I don't want @qliu21 and @tugluk to be burdened with approving every little change.

[qliu21]

@ben-e-whitney I have no problem with #2.
@williamfgc How does ADIOS set the merging strategy? I don't have problem with either "merge pull request" or "rebase and merge".

[qliu21]

@ben-e-whitney I created a rule for branch "ornl" - you should be able to approve pull requests there.

[ben-e-whitney]

@qliu21 I'm having trouble pushing to ornl. I know I was able to previously, but now I get the following when attempting to push.

remote: Resolving deltas: 100% (24/24), completed with 15 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/ornl.
remote: error: Required status check "continuous-integration/travis-ci" is expected. At least 1 approving review is required by reviewers with write access.
To https://github.com/CODARcode/MGARD.git
 ! [remote rejected] ornl -> ornl (protected branch hook declined)
error: failed to push some refs to 'https://github.com/CODARcode/MGARD.git'

It's protected again, or maybe it's a CI problem? (Sorry – I've never used CI before.) After thinking about it a bit I'm now leaning towards @NAThompson's idea in #34: just three (persistent) branches, master, develop, and experimental (ideally one of which I could push freely to). But we can wait to see what @williamfgc says about merging.

[williamfgc]

we do pull request based merge, basically you push to your fork and then do a pull request on github. It will give your peers a chance to review the changes and run the CI before merging into master (or you preferred main line branch). Here is our docs.

[NAThompson]

FWIW I’d suggest only requiring consensus during a merge to master. This helps people get early feedback on their branches and makes sure backups happen often.

But note that William’s suggestion is equivalent, but it’s on your fork.

[williamfgc]

Yeah, that's just what works for us. I think you guys need to come up and agree with what makes sense for your process flow. Our experience is that it took a few iterations so everyone is used to the process. Cheers!

[qliu21]

@ben-e-whitney Sorry I have been busy recently and forgot to check the status of this. Are there still any issues with this? Were you able to push to the ornl branch from your own fork?

[ben-e-whitney]

Not a problem. No, I still get the same message. That's when I try to push directly to the branch, though. Do I only have permission to instead merge pull requests from a fork? I can just work on my own fork and periodically merge if that's the case.

[qliu21]

@ben-e-whitney What I did is that other folks must create a pull request, but you should be able to push directly (since I created a rule that you are the exception)....I just further disabled all restrictions on the ornl branch, given that we have a small team and managing the ornl branch should be made easier..:-)

[ben-e-whitney]

Just successfully pushed a bunch of commits. Thanks a bunch.