Page MenuHomePhabricator

Add explicit limits to unit test/lint error names
ClosedPublic

Authored by amckinley on Sep 14 2017, 6:28 PM.

Details

Summary

Fixes T12981.

See new arc lint output: P2071

See new arc unit output: P2072

Test Plan

Ran arc unit/lint/diff and observed new error instead of a Conduit error

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

amckinley created this revision.Sep 14 2017, 6:28 PM
epriestley accepted this revision.Sep 14 2017, 6:39 PM

Looks good to me, thanks!

This revision is now accepted and ready to land.Sep 14 2017, 6:39 PM

Do you still want me to add the same checks to HarbormasterBuildUnitMessage->newFromDictionary()? AFAICT, that would just move the error message from save time to creation time.

This revision was automatically updated to reflect the committed changes.

Also, I landed this onto experimental. Do we want this on master as well? If so, what's the best way to accomplish that? arc land --revision D18603 --onto master?

newFromDictionary() gets called before saving the records in the database (JSON is converted to objects with newFromDictionary(), then saved). It's maybe worth doing since it'll be a slightly nicer experience for old arc clients or manual Conduit callers, but not a big deal either way.

Easiest is probably git cherry-pick <commit> + git push, land doesn't know that you want to cherry-pick instead of rebase/merge.