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

[INFRA] Adding contributor appendix sentence to PR template #299

Merged
merged 4 commits into from
Aug 21, 2019

Conversation

franklin-feingold
Copy link
Collaborator

@franklin-feingold franklin-feingold commented Aug 12, 2019

closes #298, related to #284

sappelhoff
sappelhoff previously approved these changes Aug 12, 2019
@effigies
Copy link
Collaborator

Should we suggest people edit the wiki?

@sappelhoff
Copy link
Member

@franklin-feingold here's a git comment, unrelated to this PR:

I have seen merge commits on your PRs a couple of times:
image

While these are technically not a grave problem, they do "clutter" the git history, if somebody wants to perform a git log for example.

An easy fix to not get these merge commits is to:

  1. We always start from master: git checkout master
  2. Before hacking a new feature/fix: run git pull upstream master
    1. this requires you to have the upstream remote configured, see the release protocol
  3. Then do git checkout -b my_new_branchname
  4. Then hack whatever
  5. Then do git push -u origin my_new_branchname
  6. Then do a PR via the GitHub interface

At least this would work well from the command line. Now if you want to add a new PR, you simply start again with step 1.

If you are working with the GUI I unfortunately don't have guidance, ... but then perhaps it'd be a cool learning challenge to switch to the command line :-)

@effigies
Copy link
Collaborator

If we're talking branching/merging strategy, here's my usual approach:

  1. git fetch upstream
  2. git checkout upstream/master -b new_feature_branch
  3. git push -u origin new_feature_branch
  4. As needed, git fetch upstream && git merge upstream/master (or git rebase upstream/master, if you're feeling confident)

This is basically equivalent to @sappelhoff's, but I almost never directly check out master or git pull. I do not touch the master branch on my fork of the repository. It only leads to pain and sloppy merges.

If I'm a maintainer and occasionally need to push directly to upstream/master, then I'll set my master branch to track it:

git branch --set-upstream-to=upstream/master master

Then I'll git checkout master && git merge --ff-only upstream/master before making a change and pushing to upstream/master, but otherwise I'll never sit on a master branch. This way you never find yourself in a place where you have conflicting commits.

If I'm feeling like I want my fork's master synced with upstream/master:

git fetch upstream
git push -f origin upstream/master:master

Force pushes will help avoid any sentimental attachments to your master branch.

@franklin-feingold
Copy link
Collaborator Author

I use both (CLI and GUI), likely for quickly doing something I do a quick compare sync with my master. my git is weird sometimes and will push to master and likely break travis or circle and then a hot fix is needed. this was seen when this repo was first created. essentially the concerns Chris pointed out.

presumably if one is interested in seeing our history they would go to our changelog?

Perhaps if this is a concern it would be good to detail out how we would like contributions to be formatted and submitted?

@sappelhoff sappelhoff merged commit 91ddd33 into bids-standard:master Aug 21, 2019
@sappelhoff
Copy link
Member

I see :-) for maintainers that interact regularly with git and the spec I would advise to always use the CLI and benefit from the control it gives you. I am sure @effigies and I can help you whenever git is acting weirdly!

presumably if one is interested in seeing our history they would go to our changelog?

yes, that should suffice for 95% of all cases

Perhaps if this is a concern it would be good to detail out how we would like contributions to be formatted and submitted?

Currently I think this is something among the maintainers rather than for all contributors 🤔 but if comes up again and again, then yes - maybe guidelines would be good.

Thanks for the PR @franklin-feingold

@franklin-feingold franklin-feingold deleted the PRtemp branch August 23, 2019 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve contributor guidance
3 participants