Page MenuHomePhabricator
Create Task
Maniphest T9417

Arcanist and/or Differential should prevent code reviews from non-Phabricator repos.
Closed, ResolvedPublic
Actions

Description

We have a bunch of git repos all hosted externally. I haven't added them all to Phabricator.

I made a change in a repo that was not yet added to Phabricator - the change was to add the .arcconfig file. So Arcanist worked, but I did NOT add the repo to Phabricator with Diffusion. Then I typed arc diff ... and created a Diffusion code review in Phabricator for a repo that didn't actually exist there.

I worked fine other than there was absolutely no mention of what repo the change was for in Differential. The review noticed this and pointed out the problem to me. Arcanist/Differential should at least warn about a missing repo - we'd probably like it to completely deny the code review.

Thanks!

Event Timeline

oliver.fisher created this task.Sep 14 2015, 6:57 PM
oliver.fisher updated the task description. (Show Details)
oliver.fisher added projects: Differential, Arcanist.
oliver.fisher added a subscriber: oliver.fisher.
epriestley added a subscriber: epriestley.Sep 14 2015, 7:34 PM

I think you can write a Herald rule like this:

  • Herald → Create Herald Rule → Differential Diffs → Global

Then, write a rule to reject diffs with no repository -- something like this:

Screen Shot 2015-09-14 at 12.31.50 PM.png (777×1 px, 109 KB)

Let me know if that works?

It's an intentional feature that you can onboard to code review very gradually (e.g., start by copy-pasting arbitrary diffs with zero setup/configuration and gradually adopt more and more of the workflow as you're ready) so there aren't many options to force installs to jump right into the deep end right now. I think Herald can cover this particular need effectively, though.

oliver.fisher closed this task as Resolved.Sep 14 2015, 8:54 PM
oliver.fisher claimed this task.

Yep, works great. Thanks. Herald is nice. Will play with that some more. 😄

Phabricator · Built by Phacility