Page MenuHomePhabricator

Remove lint and unit excuses and "--advice" and "--excuse" flags from "arc diff"
ClosedPublic

Authored by epriestley on May 30 2020, 9:47 PM.

Details

Summary

Ref T13544. Long ago, "arc diff" started prompting the user to provide "excuses" when they submitted changes with failing lint or unit tests.

At the time, "arc" was generally more heavy-handed and the review workflow had fewer points where it provided feedback about lint and test issues.

As the workflow has evolved, there is now significantly more feedback (promotion behavior from Draft in Differential, warnings on "arc land", etc). These days, these prompts feel archaic and like they're just getting in the way.

When lint/unit have Harbormaster-triggered components, this prompt is also too early (since Harbormaster tests may fail or raise lint messages later). A modern version of this would look more like putting revisions in some kind of locked state until authors explain issues. It's possible that's worth building, but I'd like to see more interest in it. I suspect this feature is largely just a "nag" feature these days with few benefits.

Test Plan

Grepped for "advice", "excuse", "handleServerMessage", "sendMessage", "getSkipExcuse", "getErrorExcuse", got no hits. Generated this revision.

Diff Detail

Repository
rARC Arcanist
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 30 2020, 9:47 PM
epriestley requested review of this revision.May 30 2020, 9:48 PM
epriestley edited the summary of this revision. (Show Details)May 30 2020, 9:54 PM
This revision was not accepted when it landed; it landed in state Needs Review.Fri, Jun 5, 8:22 PM
This revision was automatically updated to reflect the committed changes.