Thanks Christian, that helps a lot, especially drawing my attention to aliases. I think that between git and rb aliases I might be able to come up with something elegant and simple.
A couple of things: Is there a way to get rbt to remove "Ship it" to address the unreviewed changes scenario that you describe? I am not unduly worried about that -- IMO that falls under "bad behavior" which we don't have much trouble with at our small size. But, an ounce of prevention... Having said that, I should point out that setting up required reviewers per file/dir is more of a check for unintended consequences than a check for bad behavior. :) Also, I have found that I get errors when passing '-u' to rbt when there is nothing to update. It would simplify automation if there were a way to get rbt to ignore use of '-u' when it's a no-op instead of erroring out. I would not be surprised to hear that it needs to be that way for some good reason, but I thought I would toss it out there since it seems like an easy thing for people to overlook in their workflows. I may be able to alias my way out of that though... Thanks again, I'm off to play with aliases. :) Dave On Wednesday, May 17, 2017 at 2:43:18 PM UTC-7, Christian Hammond wrote: > > Hi Dave, > > So I think there may be some confusion as to what `rbt land` is and what > it ultimately does, and I want to go into that to see if it clears anything > up. > > rbt land just does the following: > > 1) Makes sure the commit has a review request that has been approved > 2) Updates the commit message with the latest from the review request, and > stamping the review request (basically calling `rbt stamp`) > 3) Merges the feature branch onto the destination branch (optionally > squashing) > 4) Optionally deleting the original branch, and optionally pushing the > destination branch > > If you want to commit and land to the same branch, then `rbt land` isn't > for you. The purpose of the command is to take a change that's not on the > destination, verify that it's landable, and then get it to the destination. > The reason it enforces using a separate feature branch off the land > destination branch is to ensure that you don't accidentally push changes > that haven't been approved for review. > > Doing development on the branch that you'd then land to means the "Ship > It" checking won't be as effective. If a person accidentally git pushes the > branch, they've just put themselves and everyone else in a state where > there's code now in the repository that's either unreviewed or has issues > that still need to be addressed. > > Given that, I don't think it makes sense to add an option to rbt land for > this use case. You'd probably want something that ties together the `rbt > stamp` and `git push`. You can do this easily by defining an alias in the > repository's `.reviewboardrc` that hooks those operations together. > > RBTools has no automation around branch hierarchy. `rbt land` doesn't care > about hierarchy. It's going to land the branch you specify at the target > you choose. RBTools 0.8 will have some stuff for auto-determining the > nearest tracking branch, but that's it. > > Christian > > -- > Christian Hammond > President/CEO of Beanbag <https://www.beanbaginc.com/> > Makers of Review Board <https://www.reviewboard.org/> > > On Wed, May 17, 2017 at 2:24 PM, <[email protected] <javascript:>> > wrote: > >> I've spent quite a bit of time playing with this and while I think I am >> getting closer to a usable approach I still have some concerns. >> >> As an aside, the following example from the workflow blog post creates a >> merge conflict: >> >> $ git checkout master >> $ rbt land --dest=master --push my-branch-1 >> $ rbt land --dest=master --push my-branch-2 >> >> The two land commands should be performed in the reverse order, as far as >> I can tell. >> >> Anyway, on to the workflow that I am trying to mold and shape from my >> end, as I try to understand how to mold and shape RB to fit. I am starting >> to think this is a square peg/round hole problem but since I really want >> the file-level reviewers functionality(to ultimately extend to required >> reviewers) I persist. :) >> >> Your comment "You wouldn't be committing and landing to the same >> location." seems to advise against using land at all. Here is the current >> workflow for the most common use case: >> >> >> 1. git checkout master >> 2. git fetch && git pull >> 3. git checkout -b working-branch >> 4. make some changes >> 5. git commit >> 6. git push --set-upstream origin working-branch (or otherwise push >> to remote branch, usually this branch has same name as the JIRA tracking >> the issue/task) >> 7. Make any changes to the draft pull request description/reviewers, >> then click "Create pull request" button. >> 8. Address any review comments iteratively with edit/commit/push >> 9. Once approvals are in place, merge to master. >> >> >> Alternatively, a branch off of master may be created as a remote "feature >> branch", and devs will create their own branches off of that and we simply >> add another level of the above steps, similar to what you described. >> >> So, what I am looking to do is insert RB around steps 6..8, and find a >> way to automagically add reviewers based on what files have been changed. >> My thinking was that we could create a special user in Bitbucket (e.g. >> "ReviewBoard") and preventing merge to master unless that reviewer has >> approved. This approval would be triggered by all required reviewers >> registering "Ship it" in RB. I'm pretty sure I know how to write the needed >> BB plugin and RB extensions for those integration details but certain >> aspects of integrating RB into our workflow are giving me trouble. >> >> We have explored building a wrapper script that detects when rbt should >> have the -s and -u flags applied and I had posted a while back about >> getting the stamp in place on the first push. I solved that by doing a push >> with --set-upstream immediately after creating the branch prior to any >> commits. >> >> The challenge for me at this point seems to be coming up with an RB >> workflow that supports our push to a remote branch that has the same name >> as the local branch and is NOT master. I think that the fundamental >> requirement of "land" needing to land to a different branch precludes >> leveraging that. >> >> Does RBTools have any automation around determining what the branch >> heirarchy is, short of using "land"? Is there a simple patch I could apply >> to land.py to allow landing on the same branch? >> >> It's beginning to look like using RB in our workflow will be difficult to >> automate; I was hoping to make it transparent to users but that's seeming >> less and less likely. Any help you can offer to help simplify this >> transition would be appreciated. >> >> On Thursday, May 11, 2017 at 2:28:46 PM UTC-7, Christian Hammond wrote: >>> >>> Hi Dave, >>> >>> You don't have to merge to master. You can set LAND_DEST_BRANCH in >>> .reviewboardrc to point to any branch you want. >>> >>> Any commits you're working on must be done on a feature branch that >>> comes off the branch you want to land on. You wouldn't be committing and >>> landing to the same location. For instance, this is how your branches might >>> look: >>> >>> o [master] [origin/master] >>> | >>> | o my-feature-branch >>> | | >>> | o [my-land-dest] [origin/my-land-dest] >>> |/ >>> | >>> >>> You'd set LAND_DEST_BRANCH to "my-land-dest", and what you'd be pushing >>> upstream. Code in development that would go up for review would be >>> committed to "my-feature-branch" or equivalent. It would only end up on >>> "my-land-dest" when it's been reviewed and ready to land (rbt land would >>> take care of this). That's the key thing. The upstream branch would be >>> treated similarly to the way you're treating "master" today. You'd let `rbt >>> land` manage commits going on that branch. >>> >>> Since you're not working off of master itself, you're also going to want >>> to set TRACKING_BRANCH to "origin/my-land-dest". This is used to tell >>> RBTools what the nearest tracking branch would be for the purpose of >>> generating diffs and parent diffs. >>> >>> So your .reviewboardrc would be: >>> >>> REVIEWBOARD_URL = "https://yourserver.example.com"; >>> REPOSITORY = "<Your Repository Name in RB>" >>> LAND_DEST_BRANCH = "my-land-dest" >>> TRACKING_BRANCH = "origin/my-land-dest" >>> >>> Christian >>> >>> >>> -- >>> Christian Hammond >>> President/CEO of Beanbag <https://www.beanbaginc.com/> >>> Makers of Review Board <https://www.reviewboard.org/> >>> >>> On Wed, May 10, 2017 at 3:20 PM, <[email protected]> wrote: >>> >>>> Referring to this article: >>>> http://blog.beanbaginc.com/2015/01/26/an-effective-rbtools-workflow-for-git/ >>>> >>>> What I need is to be able to land changes not in master but in a remote >>>> branch off of master. Our release process merges these branches into >>>> master >>>> via pull requests at the behest of DevOps, not the individual developers. >>>> Landing in master is not an option, because there is a qualification >>>> process that needs to take place prior to merge to master. >>>> >>>> I'm pretty sure there is something I'm not understanding about Git >>>> and/or rb, I'm posting here in case someone can help me spot the issue. >>>> >>>> When I look in the BitBucket webUI, oddly I only see the mb-1 commits >>>> on the "commits" page(none of the mb-2 commits) and I see the mb-2 branch >>>> on the branches pagem but not the mb-1 branch. The mb-1 commits are in the >>>> mb-2 branch. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (master) >>>> $ git checkout -b mb-1 >>>> Switched to a new branch 'mb-1' >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ git push --set-upstream origin mb-1 >>>> Total 0 (delta 0), reused 0 (delta 0) >>>> remote: >>>> remote: Create pull request for mb-1: >>>> remote: https:// >>>> bitbucket.org/.../rb-test/pull-requests/new?source=mb-1&t=1 >>>> remote: >>>> To [email protected]:shipwire/rb-test.git >>>> * [new branch] mb-1 -> mb-1 >>>> Branch mb-1 set up to track remote branch mb-1 from origin. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ vi foo.py >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ vi bar.py >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ git commit -a >>>> [mb-1 216b719] Adding via mb-1 >>>> 2 files changed, 4 insertions(+) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ git checkout -b mb-2 >>>> Switched to a new branch 'mb-2' >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git status >>>> On branch mb-2 >>>> Untracked files: >>>> (use "git add <file>..." to include in what will be committed) >>>> >>>> find_gitpush_done >>>> ../conf/file.txt >>>> ../copy_merchant_output.txt >>>> ../copy_merchant_output2.txt >>>> ../output.txt >>>> ../output2.txt >>>> >>>> nothing added to commit but untracked files present (use "git add" to >>>> track) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git push --set-upstream mb-1 mb-2 >>>> fatal: 'mb-1' does not appear to be a git repository >>>> fatal: Could not read from remote repository. >>>> >>>> Please make sure you have the correct access rights >>>> and the repository exists. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git push --set-upstream origin/mb-1 mb-2 >>>> fatal: 'origin/mb-1' does not appear to be a git repository >>>> fatal: Could not read from remote repository. >>>> >>>> Please make sure you have the correct access rights >>>> and the repository exists. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git push --set-upstream origin mb-2 >>>> Counting objects: 5, done. >>>> Delta compression using up to 8 threads. >>>> Compressing objects: 100% (5/5), done. >>>> Writing objects: 100% (5/5), 451 bytes | 0 bytes/s, done. >>>> Total 5 (delta 4), reused 0 (delta 0) >>>> remote: >>>> remote: Create pull request for mb-2: >>>> remote: https:// >>>> bitbucket.org/.../rb-test/pull-requests/new?source=mb-2&t=1 >>>> remote: >>>> To [email protected]:shipwire/rb-test.git >>>> * [new branch] mb-2 -> mb-2 >>>> Branch mb-2 set up to track remote branch mb-2 from origin. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ gitk >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ vi foo.py >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git commit -a >>>> [mb-2 ec57060] Add via mb-2 >>>> 1 file changed, 2 insertions(+) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git checkout mb-1 >>>> Switched to branch 'mb-1' >>>> Your branch is ahead of 'origin/mb-1' by 1 commit. >>>> (use "git push" to publish your local commits) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ rbt post >>>> Review request #71 posted. >>>> >>>> http://rb.tools.aws.....com/r/71/ >>>> http://rb.tools.aws.....com/r/71/diff/ >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ git checkout mb-2 >>>> Switched to branch 'mb-2' >>>> Your branch is ahead of 'origin/mb-2' by 1 commit. >>>> (use "git push" to publish your local commits) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ rbt post >>>> Review request #72 posted. >>>> >>>> http://rb.tools.aws.....com/r/72/ >>>> http://rb.tools.aws.....com/r/72/diff/ >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ git checkout mb-1 >>>> Switched to branch 'mb-1' >>>> Your branch is ahead of 'origin/mb-1' by 1 commit. >>>> (use "git push" to publish your local commits) >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ rbt land --dest=mb-1 --push mb-1 >>>> ERROR: The local branch cannot be merged onto itself. Try a different >>>> local branch or destination branch. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-1) >>>> $ rbt land --dest=origin/mb-1 --push mb-1 >>>> Merging branch "mb-1" into "origin/mb-1" >>>> Deleting merged branch "mb-1" >>>> Pushing branch "origin/mb-1" upstream >>>> ERROR: Could not pull changes from upstream. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin ((6efeb87...)) >>>> $ rbt land --dest=mb-1 --push origin/mb >>>> Failed to execute command: ['git', 'rev-parse', 'origin/mb'] >>>> origin/mb >>>> fatal: ambiguous argument 'origin/mb': unknown revision or path not in >>>> the working tree. >>>> Use '--' to separate paths from revisions, like this: >>>> 'git <command> [<revision>...] -- [<file>...]' >>>> >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin ((6efeb87...)) >>>> (arg: 2) rbt land --dest=mb-1 --push origin/mb >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin ((6efeb87...)) >>>> >>>> $ rbt land --dest=mb-2 --push origin/mb-2 >>>> Merging branch "origin/mb-2" into "mb-2" >>>> Failed to execute command: ['git', 'commit', '-m', u'Adding via >>>> mb-1\n\nReviewed at http://rb.tools.aws.....com/r/7 >>>> 1/', u'--author="Dave Anderson <David....com>"'] >>>> On branch mb-2 >>>> Your branch is ahead of 'origin/mb-2' by 1 commit. >>>> (use "git push" to publish your local commits) >>>> Untracked files: >>>> bin/find_gitpush_done >>>> conf/file.txt >>>> copy_merchant_output.txt >>>> copy_merchant_output2.txt >>>> output.txt >>>> output2.txt >>>> >>>> nothing added to commit but untracked files present >>>> >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin (mb-2) >>>> $ rbt land --dest=origin/mb-2 --push mb-2 >>>> Merging branch "mb-2" into "origin/mb-2" >>>> Deleting merged branch "mb-2" >>>> Pushing branch "origin/mb-2" upstream >>>> ERROR: Could not pull changes from upstream. >>>> >>>> Dave-PC MINGW64 ~/Vagrant/sws-vagrant/src/rb-test/bin ((ca80b47...)) >>>> $ >>>> >>>> >>>> -- >>>> Supercharge your Review Board with Power Pack: >>>> https://www.reviewboard.org/powerpack/ >>>> Want us to host Review Board for you? Check out RBCommons: >>>> https://rbcommons.com/ >>>> Happy user? Let us know! https://www.reviewboard.org/users/ >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "reviewboard" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >> Supercharge your Review Board with Power Pack: >> https://www.reviewboard.org/powerpack/ >> Want us to host Review Board for you? Check out RBCommons: >> https://rbcommons.com/ >> Happy user? Let us know! https://www.reviewboard.org/users/ >> --- >> You received this message because you are subscribed to the Google Groups >> "reviewboard" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> For more options, visit https://groups.google.com/d/optout. >> > > -- Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/ Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/ Happy user? Let us know! https://www.reviewboard.org/users/ --- You received this message because you are subscribed to the Google Groups "reviewboard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
