On 11/23/09 2:05 PM, "David Kastrup" <d...@gnu.org> wrote:
> Nicolas Sceaux <nicolas.sce...@gmail.com> 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: lilypond-devel@gnu.org
> Tree status URL:
> ViewVC URL:
>
> d...@lola:/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?
>
> $
>
> Does not look really convincing.
>
>
> d...@lola:/usr/local/tmp/lilypond$ git-cl rebase
> Can't locate SVN/Core.pm in @INC (@INC contains: /opt/git/share/perl/5.10.0
> /etc/perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0
> /usr/lib/perl5 /us
>
>
> Really, that's not something that would appear to helpmy workflow.
>
>
> git-checkout -d mad-dak
>
> then piping the four postings through git-am should do the trick for
> reviewing. Feel free to post the thing to Rietveld yourself if you feel
> like it, but at the moment I really can't be bothered fighting some
> none-working tools.
>
>> I think somewhere you missed a change from
>> define-builtin-markup-list-command to define-markup-list-command.
>
> I did not change any define-builtin-markup-list-command to
> define-markup-list-command in this patch series.
>
>> It would be easier to read and comment on retvield, together with
>> other modified files.
>
> Feel free to put it there.
>
>> It also seems that ly/markup-init.ly is impacted, but I don't see the
>> patch.
>
> It isn't impacted. The patch series gives the *-builtin-* commands a
> different syntax that is upwards-compatible to the user counterparts.
> It does not change the user counterparts at all.
>
> The first two patches in the series are cleanups and would work
> independently of the rest of the patch series. I had posted them
> previously on the list and they have basically been ignored. They are
> in the patch series because they touch code _overlapping_ with patch 4,
> and it makes no sense in not doing the cleanup.
>
> 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.
Thanks,
Carl
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
