lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Issue 3718 in lilypond: Patch: Web: Reword contactUsAbout macro


From: Urs Liska
Subject: Re: Issue 3718 in lilypond: Patch: Web: Reword contactUsAbout macro
Date: Fri, 20 Dec 2013 11:09:03 +0100
User-agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

Thanks for the explanations.
Basically everything is like I would have expected, but I think I will try to add some explanations to CG when I've time.

Urs


Am 20.12.2013 11:00, schrieb David Kastrup:
Urs Liska <address@hidden> writes:

Am 19.12.2013 11:32, schrieb address@hidden:
Updates:
      Labels: -Patch-countdown Patch-push

Comment #4 on issue 3718 by pkx166h: Patch: Web: Reword contactUsAbout
macro
http://code.google.com/p/lilypond/issues/detail?id=3718

Patch counted down - please push


What should I do now?
The CG says I should "send the patch to my mentor", but obviously this
isn't applicable.

And I have a few more general questions that don't seem to be covered
in the CG:

1)
origin/master has moved in the meantime. I can rebase my patch without
conflicts in this case, but what is the general task at this point?

- rebase my branch on origin/master
- if that works, create a patch with patch-format

It it doesn't work, fix it.  It is a good idea at least to run "make
all" afterwards to make sure that the Texinfo syntax is in proper order
(the much more expensive "make doc" also makes sure the syntax inside of
@lilypond fragments is ok).

- send it - where?

You can try me.

2)
What to do if the rebase would require changes to resolve conflicts?
Wouldn't the changed patch have to be reviewed first?
(possibly causing a loop with the next rebase ...)

It depends on the nature of the changes.  If they are basically all
"clerical", just check "make all" afterwards, and push.  If it means
that you need to adapt the logic of the patch, you should do a new
review cycle.  Hopefully, that's rare.

I tend do get those kinds of conflicts with my own work and tend to
resolve those mostly reviewless, but then the overlaps tend to be rather
minor.  The issues 3727/3728 were such a case.

The big bad ugly rebases are typically when your patch involves running
convert-ly, and the "Patch-push" LilyPond version is different from when
you started.  That basically means a git rebase -i, disentangling the
convertrules.py in case there were more than one for either affected
version, dropping the update-with-convert-ly commit and regenerating it.

3)
What to do if my branch contains more than one commit?
Should I squash them so the patch is one (big) commit?

No.  Clean them up so that they are logically separate and each commit
should leave the tree in a compilable state.  If you are not perfectly
sure that every commit will compile (for technical reasons, or because
only the end result got tested), create a merge commit (see
<URL:http://code.google.com/p/lilypond/issues/detail?id=3723#c7> for a
recent example).

In general, you need git merge --no-ff in order to create such a
"trivial" merge commit.  That's best practice.  Not everybody does it
like that, but in case of later problems, it makes it much easier to
pinpoint the problem and possibly do a fast fix by reverting just a
particular commit.

I wouldn't like that, for example because I would separate commits
that move stuff (e.g. to other website nodes) from commits that change
text (so translators have easier work)?

3b)
Commit messages haven't been part of the review process, right?

When using git cl, the original commit messages are what is proposed as
your git cl text.  From there, they will make it into the Rietveld
review and the review announcement.

This announcement/review text is not updated automatically after the
first "git cl upload" even if you change the commit messages.  If you
want them updated because they have become out of whack disturbingly,
you have to first change the issue description on Rietveld, then do the
git cl upload.

So basically I'm responsible that commit messages of my (multi-commit)
patch are accurate. Is that right?

Mostly yes.

If yes, I think that should be documented in the CG (I can do a few
improvements in the CD, along getting acquainted with the process)

Feel free to do so.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]