I've worked at AWS and Google. CRUX and Critique have similar functionalities. In general there's mostly a 1:1 mapping of equivalent internal tools between the two companies, albeit each with their own flavor.
Having only ever used CRUX (and ReviewBoard prior to it), I'm honestly surprised that most of what is discussed in the article is not common outside of Amazon.
Having used CRUX (and ReviewBoard prior to it), I'm honestly surprised it's considered an okay solution? Maybe 10 years ago it was fine, but the productivity it zaps and bad habits it encourages are a staggering waste of developer time. GitHub/GitLab is light years ahead, to the point where even small shops (can) have better code review process IMO. (Gerrit is decent, too, but the weak UI and interesting process cancels out other advantages is has over GitHub/GitLab.)
What were the bad habits in code review that you were thinking about? I'm stuck with TFVC and the code review in TFS is very poor but I used ReviewBoard about 8 years ago and thought it was the best F/OSS available at the time.
mainly for CRUX and the surrounding ecosystem: it's very hard to protect the main branch from pushes that don't meet approval, that's done afterwards during deployments. this is obviously and horrendously dumb, you really want to catch stuff before it goes in master/mainline, and having to fix this stuff before deploying it also a great use of time /s.
but it gets worse. the UI can't or won't do trivial rebases. so either you upload a new, rebased revision, and get the reviewers to re-review (because stale reviews get wiped), or just rebase, push, and mark as merged. i get you are supposed to review rebases, but this can grind a team to a halt. you're waiting for approvals, but by the time you get them someone else has pushed a new change... a colossal waste of time (edit: especially if you require 2 approvals, which i do think is good practice, and some AWS teams do it).
this also means people are trained to ignore the merge button or expect it to be greyed out. but it might be greyed out for other reason though. in GitHub, you have big green check-marks or red icons to indicate which conditions have and haven't been met (reviewers, CI, merge conflicts). in CRUX, hovering over the greyed out merge button gets you one cause at best (there is a page/tab that shows more detail, but almost nobody looks at it).
obviously it depends on your team, but it requires near constant vigilance to maintain high quality with CRUX, especially with a big team or less experienced devs contributing. process is supposed to make quality easier, not harder!
there are many other smaller issues, but that's the big one IMO. as for ReviewBoard, personally i prefer Gerrit, which itself doesn't have a great UI, but makes up for it in the workflow.
But you actually can setup rules to prevent merges unless certain people/groups have approved. It sounds like you and your team are suffering more from bad code review habits than a bad tool. Devs should review what they are publishing to their peers at least once before actually hitting the publish button. It’s a courtesy to them and their time
I personally love the Crux tool and some of its add ons.
> But you actually can setup rules to prevent merges unless certain people/groups have approved.
As detailed, that only prevents merging via the UX, not pushes to the main branch. Maybe the team does suck, but I still consider it a shortcoming of CRUX et al.
Not to hijack the thread too much (I'm also very curious about the answer -- I'm one of the Review Board founders and these discussions always interest me), but we support TFS these days in Review Board by way of our Power Pack add-on product, if you'd like an alternative solution. I can talk to you more about this off this thread if it's at all interesting to you.
