-
Notifications
You must be signed in to change notification settings - Fork 161
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
Add branch change tasks to allow for gitflow releasing #191
base: master
Are you sure you want to change the base?
Conversation
jakehschwartz
commented
Feb 20, 2017
- Add inquireBranches, setReleaseBranch and setNextBranch which mimic the version setting/changing commands
- Add gitflow test case
- Add inquireBranches, setReleaseBranch and setNextBranch which mimic the version setting/changing commands - Add gitflow test case
My tests fail (because I just blindly copied another test). What test framework are the tests in sbt-test package using? Does anyone have any resources that I could look at to figure out how to structure a test? |
Thanks @xuwei-k. Now that my tests pass, I still need some guidance on how make this work with hg and svn. Thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main thing missing from this PR is documentation - please update the README
to document this feature.
src/main/scala/Vcs.scala
Outdated
def setBranch(branch: String) = cmd("checkout", branch) | ||
|
||
def newBranch(branch: String) = cmd("checkout", "-b", branch) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely not the right behaviour for svn, I think svn branching requires doing an svn copy
from trunk to /branches/<branch-name>
. I'll be happy to merge if, rather than doing the wrong thing, these throw an exception to say this operation is not yet supported by svn.
src/main/scala/Vcs.scala
Outdated
def setBranch(branch: String) = cmd("branch", branch) | ||
|
||
def newBranch(branch: String) = setBranch(branch) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been too long since I've used hg to know what the right things are here. I don't think this is right though, I think branches get committed, so you would need to commit after calling the branch command. As with the svn support, happy to merge if these are changed to throw an exception saying this hasn't been implemented for hg yet.
Thanks for the review @jroper. I assumed those were not the correct implementations for hg and svn, that's why I added //FIXMEs and hopefully someone who did know what do to could help out. But if it is acceptable to just throw an exception, then I will do that, in addition to adding documentation. |
When/if someone goes to use this feature with hg/svn, it will be nice if rather than getting a nondescript error due to the wrong commands being run, they get an error that says exactly what the problem is - that is that the commands haven't been implemented. I don't think anyone is going to come and implement them for you anytime soon, I certainly don't have the time to do it, and I guess over 90% of sbt release users use git anyway, so I don't see any reason to hold back adding this feature until all backends are supported. |
… and pushed with the -u flag
@jroper I have added the documentation that you requested. I also made a major change. I realized the branch switching would not work with pushing because of the tracking remote/branch requirement. Now when creating a new branch, the tracking remote will be set the tracking remote of the current branch. This will allow us to use |
While I do not use GitFlow, being able to switch branches would be very useful to me, as my team protects the master branches of all repositories, and hence, I would like to create new release branches so they can be approved by pull request. Is any help needed with testing/reviewing/improving this PR? |
In terms of testing, this should work as described, I have been using it for several weeks. |
How exactly does this PR follow the git-flow process? If it doesnt follow it exactly the docs should be clear on that. The (git flow page)[http://nvie.com/posts/a-successful-git-branching-model/] talks about release branches and merging back to develop, which I dont see in this PR. Or is it that this just merges to a release branch and never to master. I think these are nice additions and they should not be held back, but think we should be a little clearer in the docs. Maybe call it "Simplified GitFlow". |
These changes would be very useful to me. It is possible to fix the conflicts and get this merged? |
It would be great if conflicts could be resolved !!!!! |
@jakehschwartz Thanks for the awesome improvement :) It would be really great if you could finalize this pull request! |