Development:Commit cheat sheet: Difference between revisions
David Mudrak (talk | contribs) |
sam marshall (talk | contribs) |
||
Line 25: | Line 25: | ||
== Provide clear and operational instructions to test your patch == | == Provide clear and operational instructions to test your patch == | ||
In the PULL request, please describe how the change can be tested. Please avoid vague phrases like "Make sure there is no regression in the core" or "Test all places where XXX is used". Also, try to avoid requiring resources that are really difficult to gather, if possible - as in "Use production data from a server with 100.000+ students". In most cases, the instructions for testers should go into the PULL request to make the tester's life easier. If the instructions are elsewhere (for example they were provided by the reported in the linked MDL as a part of steps to reproduce), please inform testers in the PULL request where they can find them. | In the PULL request, please describe how the change can be tested. Please avoid vague phrases like "Make sure there is no regression in the core" or "Test all places where XXX is used". Also, try to avoid requiring resources that are really difficult to gather, if possible - as in "Use production data from a server with 100.000+ students". | ||
In most cases, the instructions for testers should go into the PULL request to make the tester's life easier. This means you will need to copy and paste them into every PULL request (if there are multiple PULL requests for different branches, or if your first PULL request gets rejected). If these instructions are in the MDL | |||
If the instructions are elsewhere (for example they were provided by the reported in the linked MDL as a part of steps to reproduce), please inform testers in the PULL request where they can find them. However it is preferable to copy/paste the instructions into each PULL. | |||
It helps if you state your estimation of the testing difficulty so that testers can pick issues for them: | It helps if you state your estimation of the testing difficulty so that testers can pick issues for them: |
Revision as of 10:13, 7 April 2011
You can consider this page as a list to check before you submit a patch for inclusion into Moodle.
Split your work into a logical set of patches
Keep in mind that your commits will be reviewed before they are accepted. If the patch does one clear thing and does it well, the review process is fun. Git allows you to prepare patches on your branch into a sequence of logical steps. For example, when changing some API, divide the change into two steps. In the first commit, change the API. In the following commit, change all places that use the API.
Provide clear commit messages
Consider the commit message as an email for the developer who would explore the change in the future. We recommend to follow common Git guidelines for formatting. Include the MDL issue number and eventually the area/component at the beginning of the subject line.
MDL-xxxxx code area: short description of the patch
The subject line is followed by an empty line and then a paragraph or two of a longer description follows. This longer description is useful for issues with longer history of comments in the linked MDL so that it summarizes the patch without the need to go through the whole discussion.
Obviously, avoid messages like "as agreed in the chat" as they will become useless after a relatively short time.
Most of Git tools are optimised for this format and they can display the log of commits best then.
Names in the commit message
Retain the authorship of the patch. If the patch was submitted by someone else - for example a community member who published it in the tracker or in a forum - use the --author parameter. Make sure that your real name and contact email are recorded in patch. We use real names written in capital letters like "John Smith". Please do not use names like "john smith", "John S", "johnny7887". If you use --author parameter, apply the same rules for the name of the author. See almost any Git tutorial on how to set your name and email in the global Git configuration.
Provide clear and operational instructions to test your patch
In the PULL request, please describe how the change can be tested. Please avoid vague phrases like "Make sure there is no regression in the core" or "Test all places where XXX is used". Also, try to avoid requiring resources that are really difficult to gather, if possible - as in "Use production data from a server with 100.000+ students".
In most cases, the instructions for testers should go into the PULL request to make the tester's life easier. This means you will need to copy and paste them into every PULL request (if there are multiple PULL requests for different branches, or if your first PULL request gets rejected). If these instructions are in the MDL
If the instructions are elsewhere (for example they were provided by the reported in the linked MDL as a part of steps to reproduce), please inform testers in the PULL request where they can find them. However it is preferable to copy/paste the instructions into each PULL.
It helps if you state your estimation of the testing difficulty so that testers can pick issues for them:
- Easy (average community member should be able to test it) - can be tested pretty easily via the web interface only at a public test site
- Moderate (knowledgeable administrator should be able to test it) - requires local installation, for example to test some 1.9 -> 2.0 upgrade steps or some non-standard environment (for example MNet features, specific platform etc)
- Hard (development skills are required to test it) - for example may require data hacking at SQL level to simulate data corruption or modifying the code to reproduce the problem
Example of instructions for testers:
INSTRUCTIONS FOR TESTING (difficulty: easy, requires teacher access to a course)
1. Log in as a teacher and go to a course (you can use some of yours or restore the attached .mbz backup) 2. Turn editing mode on 3. TEST: Make sure that the control icons appear next to the activity titles 4. Turn editing mode off 5. TEST: Make sure that the control icons are not displayed now
Introducing new strings
Firstly, think twice and try to think in a non-English language. Any string you introduce is supposed to be translated by translators who usually do it for free in their own time. Do not waste their time by using get_string() for debugging messages that are likely to almost never appear. It is warmly recommended to let Helen review your strings before you submit them. This way we can keep the terminology and the style consistent. When introducing new strings, keep them alphabetically sorted. Using a self-descriptive names of the string identifier and the placeholder properties helps the translators to guess the context. Compare the following
$string['grade'] = 'Grade {$a}';
with
$string['maxgradevalue'] = 'Grade {$a->value}';
In the first case, it is pretty difficult to guess whether the "Grade" in the string is a noun (as in "Grade 12/30") or a verb (as in "Grade submission"). In many languages, the translation depends on it. The second case is more self-descriptive as it indicates that the placeholder will contain a value.
See Development:Help strings if you are introducing a new help string.
Include AMOS script in the commit if needed
If you change the identifier of a string or split a string into two forks, provide a script for AMOS in the commit message. Since Moodle 2.0, the translations are kept on separated branches again. The AMOS plugin at http://lang.moodle.org tracks the changes in string files and automatically records modifications, additions and removals of strings. Therefore strings can be re-worded freely on stable branches and should be removed from the master branch if they are not needed any more.
If you change the identifier of the string (that is the key in the $string array), move the string from one file to another or you are introducing a new string as a copy of some current one, you should provide instructions for AMOS so that the action can be applied in all language packs. That will save valuable translators' time. Instructions for AMOS are being put into the commit message of a commit that modifies the original English string files. The commit message containing such a script may look like this:
MDL-xxxxx code area: short description of the patch
It is recommended to leave a blank line between the commit message and the script block.
AMOS BEGIN MOV [configfoobar,core_admin],[foobar_desc,core_admin] CPY [submission,mod_assignment],[submission,mod_workshop] AMOS END
See Development:Languages/AMOS#AMOS_script for more details of the syntax. See the log history real examples of usage.