X-Git-Url: https://git.donarmstrong.com/?a=blobdiff_plain;f=Documentation%2Fcontributor%2Fsource-code.itexi;h=b961a82cf6c65897090fd5736731974ac19096b4;hb=16981f14a64fef5873300150da1b592139ee77b7;hp=250ad99d2e6ae476caa26d4d786b9728306ecac0;hpb=d2762a4f1add2bb04d6fc34d3c7ae03eeb7d500f;p=lilypond.git diff --git a/Documentation/contributor/source-code.itexi b/Documentation/contributor/source-code.itexi index 250ad99d2e..b961a82cf6 100644 --- a/Documentation/contributor/source-code.itexi +++ b/Documentation/contributor/source-code.itexi @@ -45,7 +45,7 @@ If you haven't already, download and install Git. @qq{Full installer for official Git} from: @example -@uref{http://code.google.com/p/msysgit/downloads/list} +@uref{https://git-for-windows.github.io/} @end example @item Other operating systems: either install @command{git} with @@ -848,7 +848,10 @@ The branches are kept for archival reasons. * The Git contributor's cycle:: * Pulling and rebasing:: * Using local branches:: -* Commits and patches:: +* Commits:: +* Patches:: +* Uploading a patch for review:: +* The patch review cycle:: @end menu @@ -940,7 +943,7 @@ refusing to pull with rebase: your working tree is not up-to-date @noindent it means that you have modified some files in you working tree -without committing changes (see @ref{Commits and patches}); you +without committing changes (see @ref{Commits}); you can use the @command{git@tie{}stash} command to work around this: @example @@ -1088,16 +1091,13 @@ to merge @code{translation} into @code{staging} whenever he has checked that @code{translation} builds successfully. -@node Commits and patches -@subsection Commits and patches - +@node Commits +@subsection Commits @menu * Understanding commits:: -* Making commits:: +* How to make a commit:: * Commit messages:: -* Making patches:: -* Uploading a patch for review:: @end menu @@ -1129,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 @@ -1235,21 +1235,41 @@ high-res images, fixed cropping on Finale example. @end example Commit messages often start with a short prefix describing the -general location of the changes. If a commit affects the +general location of the changes. + +@itemize +@item +Doc: and Doc-@var{**}: If a commit affects the documentation in English (or in several languages simultaneously) -the commit message should be prefixed with @qq{Doc:@tie{}}. If -the commit affects only one of the translations, the commit +the commit message should be prefixed with @qq{Doc:@tie{}}. If the +commit affects only one of the translations, the commit message should be prefixed with @qq{Doc-@var{**}:@tie{}}, where -@var{**} is the two-letter language code. Commits that affect the +@var{**} is the two-letter language code. + +@item +Web: and Web-@var{**}: Commits that affect the website should use @qq{Web:@tie{}} for English, and -@qq{Web-@var{**}:@tie{}} for the other languages. Also, changes -to a single file are often prefixed with the name of the file -involved. Visit the links listed in @ref{Understanding commits} -for examples. +@qq{Web-@var{**}:@tie{}} for other languages. + +@item +Changes to a single file are often prefixed with the name of the file +involved. +@end itemize +Visit the links listed in @ref{Understanding commits} for examples. -@node Making patches -@unnumberedsubsubsec Making patches + + +@node Patches +@subsection Patches + +@menu +* How to make a patch:: +* Emailing patches:: +@end menu + +@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. @@ -1289,99 +1309,48 @@ reviewed, the developers may push one or more of them to the main repository or discuss them with you. -@node Uploading a patch for review -@unnumberedsubsubsec Uploading a patch for review - -Any non-trivial change should be uploaded to our @qq{Rietveld} -code review website: - -@example -@uref{http://codereview.appspot.com/} -@end example - -@subsubheading @command{git-cl} install - -LilyDev users should skip over these @q{install} instructions. +@node Emailing patches +@unnumberedsubsubsec Emailing patches -@enumerate - -@item -Install @command{git-cl} by entering: - -@example -git clone https://github.com/gperciva/git-cl.git -@end example - -If that command fails for some reason, try this instead: - -@example -git clone git://github.com/gperciva/git-cl.git -@end example - -@item -Add the @file{git-cl/} directory to your PATH, -or create a symbolic link to the @command{git-cl} -and @command{upload.py} scripts in one of your PATH -directories (such as @file{$HOME/bin}). +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}. -In GNU/Linux you can add directories to PATH -by adding this line to a hidden file @file{.bashrc}, -located in your home directory: +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 -PATH=~/type-here-directory-containing-git-cl:"$@{PATH@}" +git config format.suffix '.patch.txt' @end example -@end enumerate - -@subsubheading @command{git-cl} configuration - -LilyDev users should perform these @q{configuration} instructions. - -@enumerate -@item -You must own a Google account login; please create one if you do not -have one already. - -@noindent -Note that a google account does not need to be a Gmail account; you can -use @emph{any} email address for your google account when you sign up. +This should cause email programs to apply the correct base64 +encoding to attached patches. -@warning{In order for @code{git-cl} to work as expected, your Google -Account Settings must have the @q{Access for less secure apps} set to -@q{Allowed}. This is normally the default setting.} +If you receive a patch with DOS instead of UNIX line-endings, it +can be converted back using the @code{dos2unix} utility. -@item -Move into the top source directory and then configure @command{git -cl} with the following commands: +Lots of useful information on email complications with patches is +provided on the Wine wiki at +@uref{http://wiki.winehq.org/GitWine}. -@item -Move into the top source directory and then configure @command{git -cl} with the following commands: -@example -cd $LILYPOND_GIT -git cl config -@end example - -For the @qq{Rietveld server} question, the default value -(@qq{codereview.appspot.com}) should be accepted by -answering with a newline (CR). +@node Uploading a patch for review +@subsection Uploading a patch for review -The @qq{CC list} question should be answered with: +Any non-trivial change should be uploaded to our @qq{Rietveld} +code review website: @example -lilypond-devel@@gnu.org +@uref{http://codereview.appspot.com/} @end example -The @qq{Tree status URL} value should be left blank. So should -the @qq{ViewVC URL} value, since it is used by @command{git cl -dcommit} which is only for repositories which use @command{git -svn} (LilyPond doesn't). - -@end enumerate - -@subsubheading Uploading patch set +You can upload a patch for review by using our custom @code{git-cl} +@q{helper-script}. This section assumes you have already installed, +updated, and configured @code{git-cl}. See @ref{git-cl}. @warning{Unless you are familiar with branches, only work on one set of changes at once.} @@ -1390,13 +1359,13 @@ There are two methods, depending on your git setup. @itemize @item -@strong{Master branch}: (easy option, and used in @command{lily-git.tcl}) +@strong{Master branch}: (easy option) If you added your patch to @code{master}, then: @example git pull -r -git cl upload origin/master +git-cl upload origin/master @end example @c Mention staging here? @@ -1414,30 +1383,39 @@ option. @item @strong{Separate branch}: (complicated option) -Ensure your changes are committed in a separate branch, which -should differ from the reference branch to be used by just the -changes to be uploaded. If the reference branch is to be -origin/master, ensure this is up-to-date. If necessary, use git -rebase to rebase the branch containing the changes to the head of -origin/master. Finally, check out branch with the changes and -enter the command: +Ensure your changes are committed in a separate branch, which should +differ from the reference branch to be used (usually +@code{origin/master}) by just the changes to be uploaded. Checkout the +branch with the changes: @example -git cl upload +git checkout some-branch-with-changes +@end example + +If the reference branch is to be @code{origin/master}, ensure that the +branch containing the changes is up-to-date with it. Use +@command{git rebase} or @command{git pull -r} to rebase the branch to +the head of @code{origin/master}. For example: + +@example +git pull -r origin master +@end example + +Finally, start the upload by entering: + +@example +git-cl upload @end example @noindent where is the SHA1 ID of the commit to be used as a reference source for the patch. Generally, this will be the -SHA1 ID of origin/master, and in that case the command: +SHA1 ID of origin/master, and in that case you can just use the command: @example -git cl upload origin/master +git-cl upload origin/master @end example -@noindent -can be used. - @end itemize First you will see a terminal editor where you can edit the @@ -1476,23 +1454,25 @@ associate the new branch with an existing Rietveld issue, the following command can be used: @example -git cl issue issue-number +git-cl issue issue-number @end example @noindent where @code{issue-number} is the number of the existing Rietveld issue. -@subsubheading Resetting git cl +@subsubheading Resetting git-cl -If @command{git cl} becomes confused, you can @qq{reset} it by +If @command{git-cl} becomes confused, you can @qq{reset} it by running: @example -git cl issue 0 +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 @@ -1509,6 +1489,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 @@ -1535,7 +1720,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:: @@ -1789,34 +1973,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 @@ -2212,9 +2368,7 @@ has, either as a complete file or as a @q{diff} or @q{patch} @subsection Installing git Obtain Git from -@uref{http://code.google.com/p/msysgit/downloads/list} (note, not -msysGit, which is for Git developers and not PortableGit, which is -not a full git installation) and install it. +@uref{https://git-for-windows.github.io/}. Note that most users will not need to install SSH. That is not required until you have been granted direct push permissions to