Differences between revisions 5 and 6
Revision 5 as of 2007-05-07 18:23:28
Size: 4474
Comment:
Revision 6 as of 2009-09-20 22:11:56
Size: 4475
Editor: localhost
Comment: converted to 1.6 markup
Deletions are marked like this. Additions are marked like this.
Line 16: Line 16:
  * Consider multiple patches to address an issue. [:IncrementalDevelopment:Incremental development] is good, smaller self-contained patches are easier to review, easier to describe, easier to spot bugs in. If your description of a patch starts to list more than two or three things you needed to change to address the overall issue, then step back and consider if breaking the fix into multiple discrete steps would be better.   * Consider multiple patches to address an issue. [[IncrementalDevelopment|Incremental development]] is good, smaller self-contained patches are easier to review, easier to describe, easier to spot bugs in. If your description of a patch starts to list more than two or three things you needed to change to address the overall issue, then step back and consider if breaking the fix into multiple discrete steps would be better.

Advice on developing and submitting patches

Some advice on what makes a good patch submission, thus more likely to be committed quickly. A summary would be: communicate, communicate, communicate!

General Patch Advice

Advice for every patch, even popular issues with a queue of developers waiting to review.

  • Share your ideas early with the list, maybe a better way exists, maybe someone will provide some information that cuts your development time in half. If you don't share then you are guaranteed no help.
  • Ask questions to derby-dev while you are working on the patch. Unsure how to throw an exception, ask the list, need to know if a method to do X already exists, ask the list. All questions are welcome.
  • Address a single issue in the patch.
  • Consider multiple patches to address an issue. Incremental development is good, smaller self-contained patches are easier to review, easier to describe, easier to spot bugs in. If your description of a patch starts to list more than two or three things you needed to change to address the overall issue, then step back and consider if breaking the fix into multiple discrete steps would be better.

  • Don't mix code cleanup with real code changes.
  • Describe the problem and how you are addressing it. It may not be obvious to reviewers what you are trying to do, even if you think it is. You may have been working and thinking about the issue twenty four hours a day, the reviewers are detached, they don't have the context you do and probably don't have the in-depth knowledge you do.
  • Describe your change. A patch by itself gives no help on where to start looking at how the problem was solved. Provide an overview or instructions on where the overview is in the patch. E.g. "please start at the javadoc for the SQLPSM class".
  • Tell the list if you think this completely solves the issue, is an incremental fix, or is just a preview or proof of concept patch. We can't read minds!
  • Say what tests were run and if they passed.
  • If relevant, include a release note in the ReleaseNoteFormat and check the "Release Note Needed" checkbox.

  • Comment your code - code needs to be understood by itself five, ten years from now when all context is lost to time.
  • Ensure code comments add value, saying why the code is doing something is more important than how it is doing something.
  • Make sure you have all the items on the DerbyContributorChecklist covered.

  • Make sure you check the "Patch Available" checkbox when you want review and uncheck it when you are reworking your patch and plan to submit a revised patch.
  • Encourage reviews, don't aim your submission an a single person, the more eyes on the code the better.

Extra Mile Patch Advice - Driving Your Patch to Commit

If you find you are still having trouble getting your patch reviewed, the following additional steps can help get your patch reviewed and then committed. For better or worse, in the open source environment, the truth is often you have to market your patch and drive it through the process.

  1. Post to the list at least every couple of weeks if you don't get review. Ask if someone is available to look at your patch and what additional information reviewers might need for review.
  2. If you are new to the project, call that out and indicate that you have filed your ICLA.
  3. Point out attractive or interesting features of your patch like new test cases covered, user scenarios that will benefit or may be impacted, relationships to other Jira issues, etc. This will get the attention of those who might not otherwise review.
  4. Run derbyall and suites.All and include your test results. Then reviewers know the review is probably smooth sailing and not a big time committment.
  5. Keep your patch up to date and ready for anyone who might be willing to pick it up to review.
  6. If you get no response from the list, encourage other developers you know and see on IRC or elsewhere to try out or review the patch. Don't focus on just committers. The committer is the last reviewer but ideally is not the first.
  7. Review or try out patches submitted by others. Somebody might review your patch in turn. This is also good for your accumulated merit and a great learning opportunity.
  8. Perform PatchListMaintenance, so the list is small and your patch stands out.

PatchAdvice (last edited 2009-09-20 22:11:56 by localhost)