reenhanced / gitreflow

Reflow automatically creates pull requests, ensures the code review is approved, and squash merges finished branches to master with a great commit message template.
MIT License
1.49k stars 64 forks source link

[Issue #66 + #74] Implementing "Git Reflow Refresh" #164

Closed simonzhu24 closed 8 years ago

simonzhu24 commented 8 years ago

Creating a new command "Git Reflow Refresh" to provide a solution to issues 73 + 66.

codenamev commented 8 years ago

@nhance and I were having a discussion this morning on the value of supporting this command, and whether or not it would be better served in just documentation.

Currently reflow is flexible and relatively lightweight with the expectation that the user knows what they are doing.

We can already achieve updating a feature branch with a single command git pull origin master. @nhance can you voice your concerns here as well?

@simonzhu24 can you explain some common scenarios where you found the need for this feature. Maybe there are use cases we aren't thinking of.

pboling commented 8 years ago

We are implementing workflows which will allow us to use a different flow strategy (interactive rebase) which will allow us to not ever have the frivolous "merge commits" you get when "updating a feature branch". As part of that we will use this intermediate "refresh" of a feature branch if we need to get new code before we "deliver" a PR.

Background: we are not going to be using your git flow process, but are hoping to not have to use a total fork of the code base. We want to make the tool flexible enough for most workflows that people use.

pboling commented 8 years ago

Of course this particular command can easily be added to git reflow as a plugin, so this is not crucial as part of the project at all,and wouldn't mean we need to fork. We absolutely will need the "workflow" ability, which I am working on.

pboling commented 8 years ago

I have just realized from reading this other issue https://github.com/reenhanced/gitreflow/issues/74#issue-48552299 that we won't care about the frivolous merge commits because we will be interactive rebasing (like squash merging) at time of delivery so the git log is still clean.

So we can merge instead of rebase, and that's fine.

Still, if this command is outside the scope of this project we can turn into into a plugin.

codenamev commented 8 years ago

We have worked on several projects where the workflows differed slightly, but enough that we weren't able to use our own tool (reflow); which is what prompted the idea.

A "refresh" of the feature branch is certainly a common practice in many workflows. Our use of "commands" though has been to try and keep them to a minimum, and use helper methods to perform specific tasks (the abstraction for this has been a slow process). My goal for this was to make it easier to eventually introduce the idea of "workflows" by bundling together sets of helper methods and letting users re-define the specifics of these with their own modules. As a very crude example, see here for a rough WIP on my first stab at it. I'm not 100% sold on my own idea for how to introduce this. I'm curious to see how you're tackling it :smile:

I think what @nhance and I were mostly debating is whether or not a "refresh" needs it's own command, or if this can just live inside the review and deliver. The footprint for this seems small enough though it may be nice to include.

nhance commented 8 years ago

Sorry guys, jumping in now in support of what @codenamev has said. The idea is that we make the assumption that the user knows git and is using reflow just to automate things related to the delivery process. With an implicit "you know what you're doing", it means that we don't want to hide you from git and expect the user to know how to use git, preferring to use reflow only to collapse commands that would normally be multiple steps inside a workflow into a single step.

Fetch and update or a pull down from origin is something git does really well and we don't see the value we gain in assigning an alias to that step.

We believe keeping reflow small and reusable is good for everyone, both new coders and cranky old programmers like @codenamev and I ;)

pboling commented 8 years ago

I like the suggestion for

git pull origin feature-branch && git pull remote base-branch
codenamev commented 8 years ago

Coming around to this idea. It would be nice when coming back to older projects to just git reflow refresh and have it sync everything.

simonzhu24 commented 8 years ago

Hey guys, since this thread is getting a bit long, I'd like some clarification here, should we just close this diff or finish the refresh feature?

I personally do see value in this feature. Since we are making the git workflow process easier, the refresh feature allows users to just merge the latest changes of the configurable base branch to their feature branch in a single git command. What normally would be 4 or more commands can be written as a single command using git reflow refresh.

I understand that you want to keep the reflow commands to a minimum, but this is a useful command that falls in line with making git users' lives easier. (And it does not require extra user knowledge about git). After reading these comments, it seems we might have different views on this issue, so maybe we can come to an agreement?

Or maybe tell me if I should continue working on this diff?

codenamev commented 8 years ago

@simonzhu24 pinging you here.

When you're actively working on a project, chances are good that your master is already up-to-date (and even if it isn't, it's usually not more than a few commits behind and the deliver command will update it before merging your feature branch). Also, it's common practice for us to just git pull origin master before submitting a review since there could be merge conflicts we'd need to resolve. It is often a good idea to push up your feature branch before running this so you can run both your feature and the update on CI separately in case the update from master introduces failures. The edge-case being multiple authors, git will likely complain that your branch is behind the remote branch when you go to push to it. The base of our concern lies in introducing a new reflow command that consists of two or less commands (in this case, it'd be better suited in a helper method).

I see value in refresh mostly in coming back to older projects. Where you can say "Hey reflow, make sure my master and feature branches are up-to-date and in sync". We often work on many different projects throughout any given month, and having a single command to ensure this would be very useful.

simonzhu24 commented 8 years ago

Updated and responded to comments! Please review when you have a chance.

simonzhu24 commented 8 years ago

@codenamev I updated the code a bit.

Changes since last time:

  1. Added function flag options -r for remote_location and -b for base_branch
  2. Removed the final git push command from git refresh workflow. Git Refresh refreshes the user's current branch based on their base base and remote. Pushing the branch afterwards is up to the user. This is simpler than asking the user to specify their destination location to push to, just in case the user wants to continue development on the branch.
  3. Fixed Git Reflow Review (There was a bug where the command line flag options -t and -m where accessed through global_options instead of options and was untested). The flag options only exist in options.
  4. Updated Rspecs to pass and docs to reflect the current changes.
codenamev commented 8 years ago

After you address those last couple comments I'll merge away :smile:

simonzhu24 commented 8 years ago

Good points in the last comment! Instead of calling update_current_branch which does a git pull and a git push, I just run command for git pull, this way the user won't attempt to push to a remote that they don't have permission to directly and doesn't require us to prompt the user for another piece of information (their forked destination).

If after the merge, they still want to push, then they can push manually themselves.

simonzhu24 commented 8 years ago

Ready for merge! ^_^

simonzhu24 commented 8 years ago

yes

codenamev commented 8 years ago

just that small wording update, other than that this LGTM! Great work here @simonzhu24 😄

simonzhu24 commented 8 years ago

giphy