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.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 16, 11:17 PM
Unknown Object (File)
Sun, Mar 3, 2:00 PM
Unknown Object (File)
Feb 13 2024, 4:12 PM
Unknown Object (File)
Feb 11 2024, 3:12 AM
Unknown Object (File)
Feb 2 2024, 9:24 AM
Unknown Object (File)
Jan 15 2024, 11:09 PM
Unknown Object (File)
Dec 27 2023, 1:25 PM
Unknown Object (File)
Dec 22 2023, 3:00 AM
Subscribers
None

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
Lint Not Applicable
Unit
Tests Not Applicable