Changes between Version 16 and Version 17 of SubmittingPatches

Timestamp:

Jul 12, 2016, 2:51:40 AM (8 years ago)

Author:

elexis

Comment:

Explain the RFC acronym. Add completeness check and jshint hint to the review process. Allow non-trivial patches to skip the rfc stage. Rephrase few things and fix typo.

SubmittingPatches

@@ -16 +16 @@
 == The [report:21 RFC queue] ==
 
-Submitting a patch is done by attaching the patch file to a Trac ticket. This can be an existing relevant existing ticket or a new ticket. To get the ticket in the [report:21 RFC queue], you should make sure the ticket looks as follows:
+Submitting a patch is done by attaching the patch file to a Trac ticket. This can be an existing relevant ticket or a new ticket.
+
+RFC stands for "request for comments".
+To get the ticket in the [report:21 RFC queue], you should make sure the ticket looks as follows:
 
  * The ''Summary'' (title of the ticket) should start with `[PATCH]`.
@@ -22 +25 @@
  * The ''Keyword list'' should have the keywords `rfc patch`.
 
-At this stage, at least one other developer should try to give some comments on the patch (on all aspects of it). It is advisable that you also drop by on [https://kiwiirc.com/client/irc.quakenet.org#0ad-dev irc], to discuss matters further. If the topic involves controversial matter (things that influence balancing f.e.), it's also best to create a forum topic for it.
+At this stage, at least one other developer should try to give some comments on the patch (most importantly whether all requirements have been identified, whether the code design is going in the right direction and how the code should be shaped to stay maintainable).
 
-Some patches can take longer to review due to high complexity or lower priority. But it's best to keep in touch with us and ask for reviews.
+It is advisable to drop by on [https://kiwiirc.com/client/irc.quakenet.org#0ad-dev irc] to discuss matters. If the topic involves controversial matter (things that influence balancing or the game design in unforseen ways), it is recommended to create a forum topic for it.
 
-Note that any developer can help in the RFC process. So if you have some time, you can also help with other patches in the queue: playtesting, checking code quality or even just seeing if it compiles on your system.
+Some patches can take longer to review due to high complexity, lower priority or other circumstances. But it's best to keep in touch with us and ask for reviews.
 
-When you're already familiar with the codebase, and have a trivial patch (that won't cause discussions on the code style or implementation of the feature), you can skip the RFC stage, and push your patch to the review queue directly.
+Note that non-team members can help in the RFC process too by playtesting, checking code quality or even just seeing if it compiles on your system.
+
+When you're already familiar with the codebase and have a patch that won't cause discussions on the code style or implementation of that feature, you can skip the RFC stage, and push the patch to the review queue directly.
 
 == The [report:15 review queue] ==
@@ -39 +44 @@
  * An in-game test (check if the patch implements the advertised changes)
  * Unit tests (check if the required tests are implemented and the tests still work)
+ * Completeness (verifying that there are no files forgotton which require similar changes)
 
-When the patch passes on all points, it gets committed. When the patch doesn't pass, it will be denied and send back to the RFC queue.
+A code quality tool like jshint is recommended to test for non-obvious syntax mistakes.
+
+When the patch passes on all points, it gets committed. When the patch doesn't pass, it will be sent back to the RFC queue.
 
 == Legal disclaimer ==