I'll look into the feedback for stamping, but it really comes down to limitations with how Git handles history rewriting. If something is pushed, it should be considered locked, or you end up impacting other users who are working with the same branch as you. Pushed commits can't be amended without essentially forking the history of that branch.
If you want the stamp URL in there, you can post for review before you push, and that'll successfully amend the commit. That's one way of doing it. However, whether that's useful really depends on how you're collaborating on changes: 1) If you're pushing the entire branch for review (containing multiple commits from multiple authors): Generally-speaking, you can ignore stamping of the commits and just use rbt land to merge the branch when the work is done. I know you said there's a different process for merging those branches though. What land would do is add a commit message based on the review request's summary/description/testing done to the merge commit, and stamp that. That way, you can see that the whole branch is reviewed there. However, the downside is that the work ends up growing and changing quite a bit, making it tricky to review and encouraging "fix it" commits. It's just a greater burden all around for reviewers, and isn't as effective as reviewing individual commits. 2) If you're pushing changes as you make them to a shared branch and reviewing those commits individually: You can stamp the commits up-front before pushing them with their respective review request URLs. However, updating the review request is hard, because now you have fragmented commits that may depend on other work that you probably don't actually want to bring in. Review Board cannot help you filter these out. Scattered diffs in a range (skipping other people's changes in-between) cannot be applied to a source tree. You can't go back and amend those commits without breaking everybody else. There's also just no way of distinguishing a reviewed commit from an unreviewed commit. So there's a trade-off here. You can review each commit in Review Board, but you can't easily track updates to those commits. 3) Alternatively, consider changing up the model to only push commits to that branch after they've been removed. This is really the best way to review changes going into any branch. Keep your work in a topic branch until you have something landable. Put that through the review process. When reviewed, land in that shared branch. This encourages clean commits, since you can amend them with fixes requested during the review process, making it easier later to track down problems or bisect the branch. It encourages smaller commits, helping with review time. It encourages faster turnaround on reviews, since others working on the branch will be eager to get your changes in. This is what we do to collaborate on changes. It works for us. Not being at your company, I can't say what's truly best for your workflow, but this process is what we've seen work in most cases, especially to help ensure code is well-reviewed. It's also not much different from even a Perforce workflow. Fundamentally, it's about trying to get readable, reviewable pieces of code into a codebase that others can make use of, free of bugs, with a guarantee of having gone through a review process. The smaller the pieces and the more engaged people are in review, the faster you can get changes landed. If you want to retain the current process, and stamping is the one bottleneck, you really don't have to stamp. It's just an aid to people down the road so that they can see "this change was reviewed here." I wish I had a better answer for you, but in the end, you're dealing with limitations in Git's idea of how history works. There's only so much that can be done to work around that. Christian -- Christian Hammond President/CEO of Beanbag <https://www.beanbaginc.com/> Makers of Review Board <https://www.reviewboard.org/> On Thu, Jan 12, 2017 at 1:24 PM, <[email protected]> wrote: > I should add, there appears to be no error message about the stamp. > > > On Thursday, January 12, 2017 at 1:08:52 PM UTC-8, [email protected] > wrote: >> >> Thanks for the info -- I have seen that workflow page before but unless >> I'm missing something that is not a viable workflow for us. Basically, any >> developer may create a local branch off of master. As I sketched out in my >> previous post, they also create a new remote branch of the same name. Note >> that we also use JIRA, and code needed to resolve a JIRA is checked in via >> a branch with the same name as the JIRA ID. They make any number of commits >> as you describe, though usually on only one branch. Then the perform a git >> push, and here is where the solution you propose becomes unworkable for us. >> >> Here is where we have a need that is apparently not considered in the >> workflow that you propose: Often times we will have more than one developer >> contributing their expertise to resolving the issue that this branch seeks >> to solve. This is why our default behavior is to "git push", since that >> allows us to either approve and merge the changes or another person can >> checkout that branch, make some changes, then commit and push them. >> >> There is a separate devops process by which these development branches >> are merged into master, so it seems that "rbt land" isn't going to help our >> development community, though I suppose it's conceivable that devops may >> benefit. >> >> I have used RB at a prior job, it integrated nicely with our Perforce >> workflow there and I came to appreciate it. However, things are different >> now. We cannot work in isolation, we need to be able to collaborate on >> changes. Can you help me understand where in the link that you posted(or in >> some other link) a viable collaborative workflow is presented? >> >> Thanks, >> >> Dave >> >> On Wednesday, January 11, 2017 at 10:05:34 PM UTC-8, Christian Hammond >> wrote: >>> >>> Hi Dave, >>> >>> I believe this is because the commit is pushed upstream, and shouldn't >>> be stamped. Stamping alters the commit and breaks history, requiring a >>> forced push and possibly affecting other users. What we recommend is to do >>> your work in a local development branch, post for review, and then when >>> it's reviewed you can land it upstream. Your commit will be properly >>> stamped and, if configured on Review Board, the review request will be >>> automatically closed. >>> >>> Do you receive any kind of error message about stamping when posting the >>> change? >>> >>> By the way, this is the workflow we use and recommend. We'll be >>> incorporating it into RBTools docs before too long. >>> >>> http://blog.beanbaginc.com/2015/01/26/an-effective-rbtools- >>> workflow-for-git/ >>> >>> The --guess-* flags are also defaults now, so you won't need to specify >>> them. Most of the rest of the arguments can be set in .reviewboardrc as >>> defaults. (rbt setup-repo will set the branch and tracking branch for you, >>> or you can specify them yourself as BRANCH and TRACKING_BRANCH.) >>> >>> Christian >>> >>> -- >>> Christian Hammond >>> President/CEO of Beanbag <https://www.beanbaginc.com/> >>> Makers of Review Board <https://www.reviewboard.org/> >>> >>> On Wed, Jan 11, 2017 at 9:51 PM, <[email protected]> wrote: >>> >>>> Workflow(from master branch @ HEAD): >>>> >>>> git branch spam >>>> git checkout spam >>>> vi eggs >>>> git add eggs >>>> git commit <- edit/save commit message >>>> git push --set-upstream origin spam >>>> rbt post -s --guess-summary --guess-description --branch spam >>>> --tracking-branch origin/master -d -p $commit_id" >>>> >>>> Now, if I go look at my commit at this point there is no stamp in the >>>> description. The minimum action that I have found to properly stamp the >>>> commit(s) from this point is the following: >>>> $ git pull --no-edit; git push >>>> >>>> The problem with this approach is that now extra commits are visible in >>>> Bitbucket. >>>> >>>> Where am I going wrong? Is there some step I missed or some set of >>>> directives I can issue through rbt to get the stamp in place without using >>>> git push? >>>> >>>> RB 2.5.7 >>>> RBTools 0.7.6 >>>> >>>> -- >>>> 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.
