Bug 227980 - Resolve land v. land-cowhand differences
Summary: Resolve land v. land-cowhand differences
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 221991
  Show dependency treegraph
 
Reported: 2021-07-15 00:15 PDT by Sam Sneddon [:gsnedders]
Modified: 2021-07-15 00:15 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Sneddon [:gsnedders] 2021-07-15 00:15:28 PDT
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.
Comment 1 Radar WebKit Bug Importer 2021-07-15 00:15:40 PDT
<rdar://problem/80617267>