Hey Dave, Glad that helped :)
On Wed, May 17, 2017 at 3:34 PM, <[email protected]> wrote: > 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... > I wasn't sure what you meant here by removing Ship It. Can you go into that more? > 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... > Hmm, errors aren't good. What are you seeing? -u has to do some guesswork to find a matching review request. What it does is check the user's pending review requests, trying to perform matches against the summary and the description. If there's a review request with an exact match, then it can feel comfortable choosing that one, but if not, it falls back to fuzzy matching with confirmation. Unfortunately, with Git, there isn't a really good, reliable, fool-proof way of storing metadata for associating things like a review request with a branch. Anyway, because it can't directly look up a matching review request through associated metadata, it has to scan and match, and that means it can't really assume that the user would want a brand new review request (there might be another you want to match that has a different summary and description from what you have locally). > > Thanks again, I'm off to play with aliases. :) > This might help a bit: http://blog.beanbaginc.com/2015/01/19/composing-workflows-using-aliases-in-rbtools/ -- Christian Hammond President/CEO of Beanbag <https://www.beanbaginc.com/> Makers of Review Board <https://www.reviewboard.org/> > > 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]> 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/201 >>>>> 5/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=m >>>>> b-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=m >>>>> b-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]. >>> 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. > -- 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.
