Proper workflow/process to do code review before change is pushed to remote repository

Question

I am new to both git and Crucible, and I am trying to figure out the best workflow/process to do code reviews.

One method I have gotten working is a code review on code that is already pushed to the remote repository, but that is a little late for doing a review.

I have a local clone of the remote repository with changes, I have tried both committing them locally or just staging them, and cannot figure out how to create a review from that. I did attempt to use the "patch" file, creating a diff of the changes, but then I get an error regarding "Crucible cannot anchor the patch to this repository".

Allan Haywood1 · Answer

Thank you everyone for your feedback. I am going to look into option 1.

As for the crucible.py, I tried using that for the pre-commit review and was getting a connectivity error.

It looks like this error only happens when the CR already exists.

$ ~/crucible.py CR-4

Crucible server: http://xxxxxxxxxxxx

Crucible username: xxxxxxxxxx

No matching FishEye repository detected Please choose a repository to anchor to, or press Enter to skip anchoring: xxx-xx

Received an unexpected response HTTP Error 400: Bad Request. Please check that http://xxxxxxxxxx is a Crucible server

I put x's for any data I didn't want to expose.

If I re-run with a new CR number (ex, the next available number CR-6), rather than the one I was trying to start, it looks to work fine.

Maciej Swinarski · Answer

Hi Allan,

One method I have gotten working is a code review on code that is already pushed to the remote repository, but that is a little late for doing a review.

The standard workflow would be to do all the work on a feature branch. You can then push it to the remote repository whenever you want.

Once all the work on the feature branch is ready you can create a review in Crucible. Select all commits that you want to review or better do a branch review. When the review is done merge the feature branch into the main branch.

Patch reviews are more usable in a centralised workflow where all the work is done on the main branch. It can be done with GIT and Crucible will handle it, but the feature branch workflow is definitely the way to it.

Best regards,

Mac

Piotr Swiecicki · Answer

Hi @Allan Haywood,

That's very typical approach when you want to get your changes reviewed before they are pushed to the production code.  Basically there are two main approaches that development teams usually would follow, pick up the one that suits you best.

* use branch branch workflow.  Basically it means you prepare your changes, push them to remote repository to a branch (so although your changes are pushed to your project repository, they are not yet merged to production branch) and get them reviewed on that branch.  You would only merge to a production branch once review is complete and reviewers approved.  
Branch creation is fast and easy in git, as is the branch merging later.
The benefit of such approach is that if you have Continuous Integration set up, the CI tool can see the changes you have pushed before they are merged so you can run builds on your changes and ensure no regressions were detected
*  Use pre commit reviews, you mentioned this approach already.  Just prepare the changes locally, once you believe they are ready for review you either create a patch file and upload it to Crucible, or use command line tool to get the diff generated and uploaded to Crucible automatically for you.  Both approaches are explained on the linked page.
Regarding the error you mentioned I can't easily help without knowing full details of the repository structure, patch contents and error details.  Crucible basically identifies the files added and modified in the patch, for the modified ones it attempts to find a repository that contain such files.  It would also use the changeset hashes if they are provided in the patch.    There might be few reasons why it struggles to anchor your changes, that may happen for example if you haven't modified any existing file, but you have added one or more new files only.  
Try using command line tool bundled in Crucible perhaps (see pre commit reviews), provide more details here or contact our support via https://support.atlassian.com/browse/CRC so we can dig into the case.

Hope that helps,
Piotr