Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords

[David Kastrup]

Carl Sorensen <address@hidden> writes:

> On 11/23/09 2:05 PM, "David Kastrup" <address@hidden> wrote:
>
>> Nicolas Sceaux <address@hidden> writes:
>> 
>>> Le 23 nov. 2009 à 01:03, David Kastrup a écrit :
>>> 
>>>> The specification of category and properties makes the *-builtin-*
>>>> variants diverge syntactically from the user specified markup.  Moving
>>>> those specifications into keyword arguments makes the builtin defining
>>>> macros upwards compatible with the user specified ones.
>>> 
>>> Could you publish your patches on retvield?
>>> It's just a matter of git-cl upload from your branch.
>> 
>> Nope.  The contributor's guide says:
>> 
>>        Then, add the git-cl directory to your PATH, or create a symbolic
>>     link to the git-cl and upload.py in one of your PATH directories (like
>>     usr/bin).  git-cl will is then configured by
>> 
>>          git-cl config
>> 
>>     and answering the questions that are asked.
>> 
>> There is no clue just _how_ one should answer the questions or what they
>> mean.  Let's see where we go:
>> 
>> $ git-cl config
>> Rietveld server (host[:port]) [codereview.appspot.com]:
>> CC list: address@hidden
>> Tree status URL:
>> ViewVC URL:
>> 
>> address@hidden:/usr/local/tmp/lilypond$ git-cl status
>> Branches associated with reviews:
>>       master: None
>> 
>> Current branch: no issue assigned.
>
> Since you haven't uploaded a patch, you don't have any reviews assigned yet.
>
> Did you try git-cl upload?

Interesting.  After all the error messages I would not have expected it
to do anything.

>> Patch 3 actually changes changes the *-builtin-* syntax and consequently
>> renders Lilypond inoperative when applied alone.  That is the core of
>> the patch series, and it is much better reviewable when kept separate
>> from patch 4.
>
> Not if you're using Reitveld with side-by-side diffs.  The two patches
> together are very nicely reviewable.
>
> IIUC, our policy is that *every* patch that is applied should result
> in a buildable LilyPond.  If not, it's a bad patch.

I don't consider this policy prudent in the particular situation "API
change implemented with little code" "Wagonloads of changes in API
users" because everything within part 1 requires an intensive review,
while the much larger part 2 can be skimmed at a much faster pace.

It would appear that git-cl contrib call has taken both commits
together.  Which is a good thing since mixing them together myself would
go against my taste.

-- 
David Kastrup