441
edits
Calendar:Hacking: Difference between revisions
Jump to navigation
Jump to search
→Creating and submitting a patch
m (→Hacking!) |
|||
| Line 112: | Line 112: | ||
4.) Go to the bug you've been working on and choose 'Create an Attachment'. Put the path to the file that CVS diff created in the proper box, and give your patch a short title describing your changes. Describe in detail the changes that you've made in the Description box. Check the 'Patch' checkbox under content type. | 4.) Go to the bug you've been working on and choose 'Create an Attachment'. Put the path to the file that CVS diff created in the proper box, and give your patch a short title describing your changes. Describe in detail the changes that you've made in the Description box. Check the 'Patch' checkbox under content type. | ||
5.) '''Important:''' Ask for a review! In the dropdown menu next to 'first-review', choose the '?', since you're asking for the review. In the textbox next to the '?' place the email address of one of the calendar peers. | 5.) '''Important:''' Ask for a review! In the dropdown menu next to 'first-review', choose the '?', since you're asking for the review. In the textbox next to the '?' place the email address of one of the calendar developers, but '''not''' one of the calendar peers. Do the same for 'second-review', but select one of the peers. | ||
Calendar peers (and therefore second-reviewers): | |||
*Joey Minta - <tt>jminta@gmail.com</tt> | *Joey Minta - <tt>jminta@gmail.com</tt> | ||
*Michiel van Leeuwen - <tt>mvl@exedo.nl</tt> | *Michiel van Leeuwen - <tt>mvl@exedo.nl</tt> | ||
*Dan Mosedale - <tt>dmose@mozilla.org</tt> | *Dan Mosedale - <tt>dmose@mozilla.org</tt> | ||
If your patch involves changes to the user interface (UI), set the 'ui-review' flag to '?' as well, and attach a screenshot (.gif, .jpg, or .png) of your proposed change. | |||
6.) Wait for the review. If you haven't heard anything in 5 days, please 'poke' the review. (Ideally, talk to them on IRC. At a minimum, post a comment in the bug.) If you haven't heard anything in 10 days, please choose a different reviewer. | 6.) Wait for the review. If you haven't heard anything in 5 days, please 'poke' the review. (Ideally, talk to them on IRC. At a minimum, post a comment in the bug.) If you haven't heard anything in 10 days, please choose a different reviewer. | ||
| Line 126: | Line 129: | ||
==== Tips for making good patches ==== | ==== Tips for making good patches ==== | ||
* Comments inside the code are encouraged! Remember, someone else is going to have to go back and read your code later without any clue of what you were thinking when you were doing it. Make sure that this can be done with as little pain as possible | * Comments inside the code are encouraged! Remember, someone else is going to have to go back and read your code later without any clue of what you were thinking when you were doing it. Make sure that this can be done with as little pain as possible | ||
* Don't just fix things, fix them correctly. If you find yourself adding lots of special cases for what seems like a simple task, the patch probably won't be approved without a very good reason. Mozilla code is complicated enough as it is | * Don't just fix things, fix them correctly. If you find yourself adding lots of special cases for what seems like a simple task, the patch probably won't be approved without a very good reason. Mozilla code is complicated enough as it is; don't make it more so. | ||
* Be careful about random whitespace changes. Don't add newlines to irrelevant areas, and make sure you remove extra newlines you added while experimenting. | * Be careful about random whitespace changes. Don't add newlines to irrelevant areas, and make sure you remove extra newlines you added while experimenting. | ||
* While alert() (and it's cousin dump()) are useful for debugging, they should not remain in code that will be checked in. Remove any alert()s you may have used for testing. | * While alert() (and it's cousin dump()) are useful for debugging, they should not remain in code that will be checked in. Remove any alert()s you may have used for testing. | ||