Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save bitoiu/0650e05f30244deaa65c to your computer and use it in GitHub Desktop.
Save bitoiu/0650e05f30244deaa65c to your computer and use it in GitHub Desktop.
Reasons why Crucible/Fisheye is better than Github (Enterprise) Pull Requests for reviewing code

GENERAL:

  • You can't include commits from different repos in a single code review using github pull requests, whereas you can with Crucible code reviews.
  • It's risky to use a pull request for code that isn't ready to be merged yet. What if someone doesn't see the "DON'T MERGE" comment/title, and clicks the big green merge button?

VIEWING:

  • You can't adjust the line width in which files are displayed, even if you make your browser window wider than the page. Some XML/HTML content extends well past 80 characters.
    • There are numerous Chrome extensions which purport to provide this feature, but I haven't been able to get any of them to work for gh:e.
  • You can't choose to ignore white space, so if a change involved indenting a lot of lines, you'll have a hard time figuring out what changed. In Crucible, you can choose to ignore white space.
  • You can't see the full file name, if the file name and path are longer than ~70 characters, and the HTML element that contains the file name is absolutely sized so you can't even expand your browser window.
    • You can hover over the file name to see the full name, but then you can only see one file name at a time.
  • No tree view to see files, so if your CR contains lots of files changing, it's a PITA to switch back and forth between them.
    • Crucible reloads the page when you view a different file, so you can use the browsers back button to quickly jump back to the last file you were viewing.
    • The github UI slows down considerably (e.g. when typing a comment) for pull requests with lots of changes (over 1K lines changing).

COMMITS:

  • If there have been multiple commits on the branch that the code review is based off of (e.g. 1 commit to start, and subsequent commits to address code review feedback), Crucible provides me with a slider to choose the exact set of (consecutive) commits I want to view. Pull Requests display all of them, or I can view one at a time.
  • Instead of displaying the actual date and time a commit was authored, Github displays "john.doe authored a month ago". That's fine some times, but other times I'd really like to know the actual date and time, because it matters.

COMMENTS:

  • Adding a new commit to a PR that changes an existing file, hides all comments for that file. So if I make 3 comments to a file in a PR giving 3 different things to fix, and the developer addresses 2 of my comments and updates the PR with a new revision of that file, I can't compare my comments to his changes. This prevents me from seeing his 2 changes in the context of my original comments about them, and also I can't see that he never addressed my 3rd comment.
    • Crucible doesn't hide a comment unless the line it is tied to is changed.
  • Top-level comments are not threaded, so you can't reply directly to a single comment. This can get confusing if there are a lot of top-level (i.e. not line) comments in the PR.

NOTIFICATIONS:

  • Email notifications are sporadic. When someone comments on a PR, I USUALLY get an email notification, but not always.
  • Github doesn't send you a notification email when a new commit is added to a PR, or when an existing commit is updated (i.e. via a force push). Unlike Crucible, Github has no built-in concept of a reviewer "finishing" his review, or tracking the percentage of files that a reviewer has seen, so it can lead to more ambiguity about the status of a code review.
  • Comment notifications don't include the previous comments in the chain, so you'll get a notification about the author saying "Sounds good, will do" and have no context of what it's about unless you open up the PR. Crucible displays the full comment thread in notification emails.

BOTTOM LINE: Crucible seems like it's been purpose-built for reviewing code. Github Pull Requests seem like they exist to support a process for sharing code between users or repos, but not for actually making code reviews easier.

However, Crucible has problems/downsides as well, as compared to pull requests:

  • The code that's reviewed in a Crucible code review is not guaranteed to be the same code that's merged to master once the review is finished. Nothing prevents the code author from making changes before or during the process of squashing the commits and pushing the single commit to master. (However, you could still use a pull request for actually merging the code, with someone performing a final once-over of the code.) This only affects usage of Crucible as a pre-commit review tool.
  • Crucible works best if you use merges to pull the latest commits from master onto the code review branch, instead of rebasing. If you rebase your branch, and then later add another commit which changes an existing file in the code review, Crucible will lose track of which file in the review is changing, and add the file again. The result is that you'll see 2 Foo.java files e.g., with pre-rebase commits on the first file, and your commit after the rebase on the 2nd file. For this reason it's recommended to only merge commits on your code review branch, and save rebasing for when you squash your commits and are ready to merge them to master (via push or pull request).
  • Crucible depends on your git repository being indexed by Fisheye. For an organization with a lot of repos, this means your personal fork is probably not going to exist in Fisheye. So in order to add your commits to Crucible, you'll first need to add each repository to Fisheye (one-time step per repo). Then you'll need to push your work branch to the main upstream repo that's indexed by Fisheye; i.e., the same repo that contains the master branch you'll eventually merge your code to. This requires every engineer who wants to create a Crucible code review to have push access to the upstream repo containing the code (because AFAICT git doesn't provide a way to grant per-branch permissions within a repo). If your organization wants to lock down permissions, and only provide push permissions to a subset of your engineers, then the other engineers will have to use pull requests instead of Crucible (or have someone else push their commits from their work branch to the main upstream repo).
  • After you push your commit(s) to the upstream repo, it will sometimes take Fisheye 5-10 minutes to add your commits to its index, which is required before you can add them to your code review in Crucible. Whereas with github, within seconds of pushing your commit(s) to your remote repo, you'll be able to create the pull request.
  • Crucible code reviews usually a little take more time to setup than pull requests. Both involve pushing your commit(s) to a remote repo, and clicking a couple of buttons on a web page. However, with Crucible you typically want to improve your code review's "objectives" section, choose 1 or more engineers to participate, and potentially add some comments about areas you want particular focus on, before starting the review. However, these extra steps can make for a better code review, IMO.
  • Crucible can be slow, when the review contains hundreds of files. However, github pull requests suffer the same problem IME.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment