]> git.donarmstrong.com Git - lilypond.git/blobdiff - Documentation/contributor/source-code.itexi
Doc: CG - Re-organize information about 'Patches'
[lilypond.git] / Documentation / contributor / source-code.itexi
index 7c4dc63778584703f17823f636e9f928483aef82..b43b3f348261813f30f0240c7667214867cc7ccb 100644 (file)
@@ -850,6 +850,8 @@ The branches are kept for archival reasons.
 * Using local branches::
 * Commits::
 * Patches::
+* Uploading a patch for review::
+* The patch review cycle::
 @end menu
 
 
@@ -1092,10 +1094,9 @@ checked that @code{translation} builds successfully.
 @node Commits
 @subsection Commits
 
-
 @menu
 * Understanding commits::
-* Making commits::
+* How to make a commit::
 * Commit messages::
 @end menu
 
@@ -1128,8 +1129,8 @@ branch are available at
 @uref{http://git.sv.gnu.org/cgit/lilypond.git/log/}.
 
 
-@node Making commits
-@unnumberedsubsubsec Making commits
+@node How to make a commit
+@unnumberedsubsubsec How to make a commit
 
 
 Once you have modified some source files in your working
@@ -1250,15 +1251,13 @@ for examples.
 @node Patches
 @subsection Patches
 
-
 @menu
-* Making patches::
-* Uploading a patch for review::
+* How to make a patch::
+* Emailing patches::
 @end menu
 
-
-@node Making patches
-@unnumberedsubsubsec Making patches
+@node How to make a patch
+@unnumberedsubsubsec How to make a patch
 
 If you want to share your changes with other contributors and
 developers, you need to generate @emph{patches} from your commits.
@@ -1298,8 +1297,37 @@ reviewed, the developers may push one or more of them to the main
 repository or discuss them with you.
 
 
+@node Emailing patches
+@unnumberedsubsubsec Emailing patches
+
+The default @code{x-diff} MIME type associated with patch files
+(i.e., files whose name ends in @code{.patch}) means that the
+encoding of line endings may be changed from UNIX to DOS format
+when they are sent as attachments.  Attempting to apply such an
+inadvertently altered patch will cause git to fail with a message
+about @q{whitespace errors}.
+
+The solution to such problems is surprisingly simple---just change
+the default file extension of patches generated by git to end in
+@code{.txt}, for example:
+
+@example
+git config format.suffix '.patch.txt'
+@end example
+
+This should cause email programs to apply the correct base64
+encoding to attached patches.
+
+If you receive a patch with DOS instead of UNIX line-endings, it
+can be converted back using the @code{dos2unix} utility.
+
+Lots of useful information on email complications with patches is
+provided on the Wine wiki at
+@uref{http://wiki.winehq.org/GitWine}.
+
+
 @node Uploading a patch for review
-@unnumberedsubsubsec Uploading a patch for review
+@subsection Uploading a patch for review
 
 Any non-trivial change should be uploaded to our @qq{Rietveld}
 code review website:
@@ -1430,7 +1458,9 @@ running:
 git-cl issue 0
 @end example
 
-@subsubheading Wait for a countdown
+
+@node The patch review cycle
+@subsection The patch review cycle
 
 Your patch will be available for reviews for the next few hours or
 days.  Three times a week, patches with no known problems are
@@ -1447,6 +1477,211 @@ Once a patch has @code{patch-push}, it should be sent to your
 mentor for uploading.  If you have git push ability, look at
 @ref{Pushing to staging}.
 
+@itemize
+
+@item
+Patches get added to the tracker and to Rietveld by the @qq{git-cl} tool, with
+a status of @qq{patch-new}.
+
+@item
+The automated tester, Patchy, verifies that the patch can be applied
+to current master.  By default, it checks that the patch allows @code{make}
+and @code{make test} to complete successfully.  It can also be configured to
+check that @code{make doc} is successful. If it passes, Patchy changes the
+status to @qq{patch-review} and emails the developer list.  If the patch
+fails, Patchy sets it to @qq{patch-needs_work} and notifies the developer list.
+
+@item
+The Patch Meister reviews the tracker periodically, to list patches
+which have been on review for at least 24 hours. The list is found at
+
+@smallexample
+@uref{http://code.google.com/p/lilypond/issues/list?can=2&q=label:patch%20patch=review&sort=modified+patch&colspec=ID%20Type%20Status%20Priority%20Owner%20Patch%20Summary%20Modified}
+@end smallexample
+
+@item
+For each patch, the Handler reviews any discussion on the tracker
+and on Rietveld, to determine whether the patch can go forward.  If
+there is any indication that a developer thinks the patch is not
+ready, the Handler marks it @qq{patch-needs_work} and makes a comment
+regarding the reason, referring to the Rietveld item if needed.
+
+@item
+Patches with explicit approval, or at least no negative comment, can
+be updated to @qq{patch-countdown}.  When saving the tracker item,
+clear the @qq{send email} box to prevent sending notification for
+each patch.
+
+@item
+The Patch Meister sends an email to the developer list, with a fixed
+subject line, to enable filtering by email clients:
+
+@example
+PATCH: Countdown to 20130113
+@end example
+
+The text of the email sets the deadline for this countdown batch.  At
+present, batches are done on Tuesday, Thursday and Sunday evenings.
+
+To create the countdown announcement, use the
+@code{make-countdown-announcement.sh} script, which takes the
+deadline date, and optionally your name.  Follow the instructions
+provided:
+
+@example
+cd $LILYPOND_GIT
+scripts/auxiliar/make-countdown-announcement.sh "Jan 1, 2001" James
+@end example
+
+The script produces an announcement that is easily readable in all
+email clients.  Also, whenever a new contributor submits a patch,
+you will be prompted to add the new username and author name to
+the script itself, and then commit those changes to the main git
+repository.
+
+
+@item
+On the scheduled countdown day, the Patch Meister reviews the
+previous list of patches on countdown, with the same procedure and
+criteria as before.  Patches with no controversy can be set to
+@qq{patch-push} with a courtesy message added to the comment block.
+
+@item
+Roughly at six month intervals, the Patch Meister can list the
+patches which have been set to @qq{patch-needs-work} and send the
+results to the developer list for review.  In most cases, these
+patches should be marked @qq{patch-abandoned} but this should come
+from the developer if possible.
+
+@item
+As in most organisations of unpaid volunteers, fixed procedures are
+useful in as much as they get the job done.  In our community, there
+is room for senior developers to bypass normal patch handling flows,
+particularly now that the testing of patches is largely automated.
+Similarly, the minimum age of 24 hours can reasonably be waived if
+the patch is minor and from an experienced developer.
+
+
+@end itemize
+
+@ignore
+There is a single Patch Meister, and a number of Patch Helpers
+(rename this?).  The list of known patches awaiting review is:
+
+@example
+@uref{http://code.google.com/p/lilypond/issues/list?can=2&q=label:patch&sort=patch}
+@end example
+
+
+@subheading Helpers: adding patches
+
+The primary duty is to add patches to the google tracker; we have
+a bad track record of losing patches in email.  Patches generally
+come to the @code{lilypond-devel} mailing list, but are sometimes
+sent to @code{bug-lilypond}, @code{lilypond-users}, or
+@code{frogs} mailing list instead.
+
+@itemize
+@item
+Unless a patch is clearly in response to an existing issue, add a
+new issue with the @code{Patch-new} label and a link to the patch
+(either on the mailing list archives or the codereview url).
+
+Issue numbers are cheap; losing developers because they got fed up
+with us losing their hard work is expensive.
+
+
+@c if we enter patches immediately, I don't think this is relevant.
+
+@item
+Before adding a patch-reminder issue, do a quick check to see if
+it was pushed without sending any email.  This can be checked for
+searching for relevant terms (from the patch subject or commit
+message) on the webgit page:
+
+@example
+@uref{http://git.savannah.gnu.org/gitweb/?p=lilypond.git}
+@end example
+
+
+@item
+If the patch is clearly in response to an existing issue, then
+update that issue with the @code{Patch-new} label and a link to
+the patch (either on the mailing list archives or the codereview
+url).
+
+@item
+After adding the issue, please send a response email to the same
+group(s) that the initial patch was sent to.
+
+If the initial email was sent to multiple mailing lists (such as
+both @code{bugs} and @code{devel}), then reply to all those
+mailing lists as well.  The email should contain a link to the
+issue you just added.
+
+@end itemize
+
+@subheading Helpers: @code{Patch-review} label
+
+The secondary duty is to do make sure that every issue in the
+tracker with a @code{Patch-review} label has passed these
+@qq{obvious} tests:
+
+@itemize
+@item
+Applies automatically to git master.
+
+It's ok to have offsets, but not conflicts.
+
+@item
+Regtest comparison looks ok; no unexpected changes.
+
+@item
+Descriptive subject line.
+
+Avoid subjects like @qq{fixes 123}; instead write @qq{Doc: discuss
+stacking-dir for BassFigureAlignment (fix 123)}.
+
+@item
+Compiles docs from scratch.  Only check this if you have reason to
+suspect it might not work.
+
+@item
+(maybe)
+
+Check code indentation and style.  This should be easier post-GOP
+when we have a better-defined code style.
+
+@end itemize
+
+
+@subheading Patch Meister
+
+The Patch Meister will:
+
+@itemize
+
+@item
+send @qq{countdown} emails to
+@code{lilypond-devel} when patches appear to be ready.
+
+@item
+send general requests to review patches, or even nasty requests to
+review patches.
+
+@item
+downgrade patches from @code{Patch-review} to
+@code{Patch-needs_work} as appropriate.
+
+@item
+downgrade patches from @code{Patch-needs_work} to
+@code{Patch-abandoned} if no actions have been taken in four
+weeks.
+
+@end itemize
+
+@end ignore
+
 
 @node Advanced Git procedures
 @section Advanced Git procedures
@@ -1473,7 +1708,6 @@ several Git branches of LilyPond source code is presented.
 * Working with remote branches::
 * Git log::
 * Applying remote patches::
-* Sending and receiving patches via email::
 * Cleaning up multiple patches::
 * Commit access::
 * Pushing to staging::
@@ -1727,34 +1961,6 @@ the patch actually @emph{adds}, like a regtest for a fixed bug, would
 get lost.  For the same reason, you should not use the git-independent
 @samp{patch} program for applying patches.
 
-@node Sending and receiving patches via email
-@subsection Sending and receiving patches via email
-
-
-The default @code{x-diff} MIME type associated with patch files
-(i.e., files whose name ends in @code{.patch}) means that the
-encoding of line endings may be changed from UNIX to DOS format
-when they are sent as attachments.  Attempting to apply such an
-inadvertently altered patch will cause git to fail with a message
-about @q{whitespace errors}.
-
-The solution to such problems is surprisingly simple---just change
-the default file extension of patches generated by git to end in
-@code{.txt}, for example:
-
-@example
-git config format.suffix '.patch.txt'
-@end example
-
-This should cause email programs to apply the correct base64
-encoding to attached patches.
-
-If you receive a patch with DOS instead of UNIX line-endings, it
-can be converted back using the @code{dos2unix} utility.
-
-Lots of useful information on email complications with patches is
-provided on the Wine wiki at
-@uref{http://wiki.winehq.org/GitWine}.
 
 
 @node Cleaning up multiple patches