Bug 227980
Summary: | Resolve land v. land-cowhand differences | ||
---|---|---|---|
Product: | WebKit | Reporter: | Sam Sneddon [:gsnedders] <gsnedders> |
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED INVALID | ||
Severity: | Normal | CC: | dpino, jbedard, ross.kirsling, webkit-bug-importer, wenson_hsieh |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Local Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | |||
Bug Blocks: | 221991 |
Sam Sneddon [:gsnedders]
From Bug 221991:
(In reply to Diego Pino from comment #22)
> (In reply to Jonathan Bedard from comment #18)
>
> > Any reason you're using `land-cowhand` over `land`? My understanding was
> > that `land-cowhand` skipped the pre-commit tests, but `land` no longer has
> > pre-commit tests in it.
>
> No other reason than tradition. When I asked other more experienced WebKit
> developers how I could land a patch without filing a bug I was taught
> `land-cowhand` and I've been keep using it since then for that kind of task.
(In reply to Wenson Hsieh from comment #25)
> FWIW, I generally prefer `land-cowhand` over `land`, since it automatically
> generates and presents a pretty-printed version of the patch that I can
> check for sanity.
Looking at the code, there's a fair difference between land and land-cowhand:
Land:
steps = [
steps.AddSvnMimetypeForPng,
steps.UpdateChangeLogsWithReviewer,
steps.ValidateReviewer,
steps.ValidateChangeLogs,
steps.Build,
steps.Commit,
steps.CloseBugForLandDiff,
]
LandCowhand:
steps = [
steps.SortXcodeProjectFiles,
steps.PrepareChangeLog,
steps.EditChangeLog,
steps.CheckStyle,
steps.ConfirmDiff,
steps.Build,
steps.Commit,
steps.CloseBugForLandDiff,
]
These are very different, and indicate very different intended uses, as far as I can tell. land is intended to be used when landing a reviewed patch, whereas land-cowhand is intended to be used when landing an unreviewed patch.
While I'm skeptical about putting much in the way of resources into this given the forthcoming transition to GitHub PRs, I suspect we should try and reduce the difference between the two (e.g. adding steps.ConfirmDiff to Land), and potentially rename land-cowhand (again) to something like land-unreviewed to make it clear what the intended use.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/80617267>
Sam Sneddon [:gsnedders]
Nobody in the past two years has complained about LandCowhand being deprecated (and we have admittedly moved to git since!), so I suggest we close this as invalid when we just remove the deprecated command.
Sam Sneddon [:gsnedders]
LandCowhand no longer exists (removed via bug 221991).