google / git-appraise

Distributed code review system for Git repos
Apache License 2.0
5.12k stars 145 forks source link

Better support the 0-state #66

Open ojarjur opened 7 years ago

ojarjur commented 7 years ago

Right now git-appraise assumes that there is an existing target ref and review ref, which have different head commits but for which there is a merge base.

Those assumptions are fine for most usage, but it is easy for someone just trying out git-appraise for the first time to not satisfy them, and the existing error message can be quite cryptic when that happens.

To improve this situation, we should change the user experience in the following situations:

  1. If the target ref (by default refs/heads/master) does not exist, but the review ref does: We should support this as a valid use case, and create a review anchored at the first (oldest) commit.
  2. If both the target ref and review ref exist, but they have no commits in common (and, thus, no merge base): This is conceptually quite similar to the first scenario. We should support this as a valid use case, and create a review anchored at the first (oldest) commit in the review ref.
  3. If neither the target nor the review ref exists: This is not a valid use case, but the current error message ("fatal: 'refs/heads/master' - not a valid ref") does not explain that. We should change this to something like "You cannot create a review in an empty repository. First, commit the changes that you want to review".
  4. If the target and review ref are the same: This is not a valid use case, but the current error message ("There are no commits included in the review request") does not explain that. We should change this to something like "Before you can create a review you need to have two different branches to compare".

There is also the scenario that the target ref exists but the review ref does not. However, that is not a valid scenario, and the current error message is probably sufficient.