[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #
From: |
David Kastrup |
Subject: |
Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords |
Date: |
Tue, 24 Nov 2009 01:03:17 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) |
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
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, (continued)
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Nicolas Sceaux, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Kieren MacMillan, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Werner LEMBERG, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Han-Wen Nienhuys, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Carl Sorensen, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords,
David Kastrup <=
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, Joe Neeman, 2009/11/23
- Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords, David Kastrup, 2009/11/24
Re: [PATCH 2/4] scm/harp-pedals.scm: Fold make-harp-pedal into \harp-pedal markup., Reinhold Kainhofer, 2009/11/23
Re: [PATCH 1/4] scm/define-markup-commands.scm: remove some unnecessary lookups, Han-Wen Nienhuys, 2009/11/22
Re: [PATCH 1/4] scm/define-markup-commands.scm: remove some unnecessary lookups, Reinhold Kainhofer, 2009/11/23