learnmath / lysa

Learn You Some Algebras for Glorious Good!
https://learnmath.github.io/
Other
16 stars 3 forks source link

Gitflow workflow #81

Closed pharpend closed 9 years ago

pharpend commented 9 years ago

I think the guide says to do the following

  1. Create a feature branch
  2. Make some edits to the feature branch
  3. Merge the feature branch into your local develop branch
  4. git pull this repo's develop branch.
  5. Send a pull request targeting develop

However, I have noticed that this doesn't work. Namely, I will do some work on a feature branch, and then some unrelated work on another feature branch. If the first set of work gets rejected, it's often the cace that the second set of work contains the first set of work (iff I merged the first set of work into my develop branch before creating the second set). This would mean that I have to do some git checkout-fu to undo the previous work. This (1) defeats the point of branches, and (2) is very difficult for someone who isn't a zen master of git.

With that in mind, I see it fitting to amend the terms to send PR's from feature branches targeting LM's develop branch.

wei2912 commented 9 years ago

I don't see why feature branches need to be merged back into develop. All my PRs are forked from branch and are merged perfectly well back into the main repository's develop. Double merging might not be a good idea.

pharpend commented 9 years ago

I agree with @wei2912

ghost commented 9 years ago

@wei2912 you're having this luck because you're working on things the rest of us aren't. The reason we merge back into a synced develop is to ensure there will be no merge conflicts. That's also why they haven't needed to be amended.

@pharpend you should be doing steps 3 and 4 in reverse order: 1) sync upstream 2) merge your branch

If I have them flipped on the contrib guide, that needs to be updated asap.

With regards to working on other features and then following the same process: any commits made to develop after the pull request is made should be automatically appended to the pull request, which will save you this problem. That's a feature of github (not git) that I don't like, but it works in your favor.

When you might have problems between two features is when a prior merge request is not accepted; this will cause all subsequent merge requests to need to be amended. That's why I'm very averse to outright rejecting a merge request (from our regulars)--I'd rather it be amended so that all subsequent merges do not have to be.

All that said, there is a time delay between a pull request and its merging. This will only get worse next week, as I go back to work; any conversation that happens over a merge is going to stretch out post-holiday season.

pharpend commented 9 years ago

@beingbrown I see, but it doesn't solve the issue I pointed out, where

My method is much cleaner and easier to understand.

ghost commented 9 years ago

from above:

When you might have problems between two features is when a prior merge request is not accepted; this will cause all subsequent merge requests to need to be amended. That's why I'm very averse to outright rejecting a merge request (from our regulars)--I'd rather it be amended so that all subsequent merges do not have to be.

pharpend commented 9 years ago

Hmm, well let's sleep on it.

ghost commented 9 years ago

I'm closing this as FIXED. So long as merges from -><origin/develop> don't conflict, I won't make a big deal of it.

ghost commented 9 years ago

I'd rather we not change the contrib guide though, because the way it's stated there will make it easiest to incorporate other users additions without making a mess of the graph.

pharpend commented 9 years ago

Yes, that's true.