Skip to content
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

Process discussion for keeping in sync with main repository #7

Open
effigies opened this issue Aug 1, 2019 · 8 comments
Open

Process discussion for keeping in sync with main repository #7

effigies opened this issue Aug 1, 2019 · 8 comments
Labels
process Related to dev process

Comments

@effigies
Copy link
Contributor

effigies commented Aug 1, 2019

Just wanted to start a discussion about how to operate a BEP like this separate from the main spec, particularly with respect to keeping things in sync so that when you're getting ready for merging back in, merge conflicts don't arise. Because this was created with the full git history, this should be fairly easy, technically speaking.

So I'm going to propose a model. Feel free to reject or adapt, but hopefully this will be helpful nonetheless.

Initial setup

I have done some initial work that I detailed in #6. That involved:

  • Pushing the current state of upstream master and common-derivatives to this repository
  • Reinstating the diffusion derivatives only in a bep-016 branch, that I propose treating as equivalent to your master branch. All PRs should go against that.
  • Rebasing the commits from [ENH] Restructuring of DWI derivatives #5 against bep-016, so that PR can be picked back up with as little interruption as possible. (@Lestropie, please ask if you need help switching to that branch.)

Maintaining

I would suggest that you identify one maintainer to periodically (perhaps weekly) update the common-derivatives and master branches, so that changes in the main spec get reflected here, and makes any potential merge conflicts evident. To set up a new repository:

git clone [email protected]:<USER>/bids-specification.git
cd bids-specification
git remote add bids [email protected]:bids-standard/bids-specification.git
git remote add bep16 [email protected]:bids-standard/bids-bep016.git
git fetch --all
git branch --set-upstream-to=bids/master master
git checkout -t bids/common-derivatives

To update:

git fetch bids bep16
git push bep16 bids/master:master
git push bep16 bids/common-derivatives:common-derivatives

As long as the master and common-derivatives stay in sync, this should work smoothly. If things get messed up, I'm happy to help figure out what happened and get things back to a clean state.

There's nothing wrong with multiple maintainers, but assigning one person (or one person at a time) might reduce the likelihood of messes.

Contributing

Otherwise, I would recommend you treat this like a normal repository, but with PRs against the bep-016 branch.

Merging back

When this PR is in a state where you're ready for final review to merge back into the primary spec, I would recommend the maintainer do:

git fetch bids bep16
git checkout -t bep16/bep-016
git push bids bep-016

This assumes you don't have a copy of the bep-016 branch already on your local repository and that you have write access to the main repository.

@effigies
Copy link
Contributor Author

effigies commented Aug 1, 2019

@sappelhoff
Copy link
Member

I would recommend you treat this like a normal repository, but with PRs against the bep-016 branch.

One could change a GitHub setting to make the bep-016 branch the new "Default" branch, perhaps that'd help

I see you already did that :-)

@franklin-feingold
Copy link
Contributor

cc @arokem

@Lestropie
Copy link
Collaborator

RE "Merging back": Would it not be preferable to create a PR on the main BIDS repository that pulls from the relevant branch on this fork, rather than explicitly pushing that branch to the main BIDS repository (which, as you say, requires write access there) prior to creation of the PR?


This information probably also needs to go into a contributing guidelines file, so that anybody outside of the main development group can more easily find the relevant information. I just wrote one for MRtrix3, so I can probably cut that down and modify to suit.

@Lestropie
Copy link
Collaborator

@oesteban @francopestilli: How much time & effort do you think you will be contributing to this fork ongoing? Would you prefer if I were to take on more of a maintainer role?

@oesteban
Copy link
Collaborator

@Lestropie I'm doing some housekeeping now, but I am taking a break the next two weeks. Totally cool if you want to take on the maintainer role :)

@effigies I believe your first code snippet should be amended:

git clone [email protected]:<USER>/bids-specification.git
cd bids-specification
git remote add bids [email protected]:bids-standard/bids-specification.git
git remote add bep16 [email protected]:bids-standard/bids-bep016.git
git fetch bids
git branch --set-upstream-to=bids/master master
git checkout -t bids/common-derivatives

@effigies
Copy link
Contributor Author

Yes, I think you're right that we need to add a fetch to that first section.

@effigies
Copy link
Contributor Author

@Lestropie Feel free to add this to CONTRIBUTING.md, if it seems good to you. When it comes time to merge.

Also, IMO, if somebody maintains a BEP repository to the point of being ready to re-merge, then I think they should be given permissions to push that branch directly to the main repository.

The nice thing about pushing to the main repository is that we can set up ReadTheDocs to show a rendered copy that updates as we make final changes, rather than digging through Circle artifacts.

@arokem arokem pinned this issue Nov 28, 2022
@PeerHerholz PeerHerholz added the process Related to dev process label Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Related to dev process
Projects
None yet
Development

No branches or pull requests

6 participants