Some factual corrections and some updates to the coding style
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2006 17:31:10 +0000 (17:31 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2006 17:31:10 +0000 (17:31 +0000)
document to reflect our current practices.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@12736 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitSite/coding/coding-style.html
WebKitSite/coding/contributing.html

index 78fc3fb6561055a67e5b4cdc5b4aac6e5872b987..16a1afd1dc97c218f80780f8a79bbacee39e7f94 100644 (file)
@@ -20,7 +20,9 @@
 
 <ol>
 <li> Use spaces to indent. Tabs should not appear in code files (with
-     the exception of files that require them e.g. Makefiles).
+     the exception of files that require tabs, e.g. Makefiles). We have
+     a Subversion pre-commit script that enforces this rule for most
+     source files, preventing commits of files that don't follow this rule.
 
 <li> The indent size is 4 spaces.
 
@@ -31,53 +33,43 @@ spaces.
 <h3>Braces</h3>
 
 <ol>
-<li> Function definitions - open and close braces should be on lines by themselves. Do not put the open brace on the same line as the function signature. For example:
+<li> Function definitions &mdash; open and close braces should be on lines by themselves. Do not put the open brace on the same line as the function signature. For example:
 
 <pre>
     RIGHT:
-    
         void foo()
         {
         }
-        
-        
+
     WRONG:
-    
         void foo() {
         }
-
 </pre>
 
-<li> Loop control structures, including for, while and do statements - the open brace should go on the same line as the as the control structure. 
+<li> Loop control structures, including for, while and do statements &mdash; the open brace should go on the same line as the as the control structure. 
 
 <pre>
     RIGHT:
-    
         for (int i = 0; i < 10; i++) {
         }
                 
     WRONG:
-    
         for (int i = 0; i < 10; i++) 
         {
         }
-
 </pre>
 
-<li> If/else statements - as above, but if there is an else clause, the close brace should go on the same line as the else.
+<li> If/else statements &mdash; as above, but if there is an else clause, the close brace should go on the same line as the else.
 
 <pre>
     RIGHT:
-
         if (timeToGetCoffee) {
            buyCoffee(&coffee);
             chugIt(coffee);
         } else if (timeToGoHome)
             outtaHere = true;
 
-
     WRONG:
-
         if (timeToGetCoffee)
         {
            buyCoffee(&coffee);
@@ -91,18 +83,17 @@ spaces.
         } 
        else if (timeToGoHome) 
             outtaHere = true;
-
+</pre>
 </ol>
 
 <h3>Parentheses</h3>
 
 <ol>
 
-<li>Function declarations and calls - do not use any spaces between the name and the open paren, inside the parentheses, or before commas that separate arguments. Do use a single space after commas that separate arguments.
+<li>Function declarations and calls &mdash; do not use any spaces between the name and the open paren, inside the parentheses, or before commas that separate arguments. Do use a single space after commas that separate arguments.
 
 <pre>
     RIGHT:
-
         int myFunction(int arg1, float arg2);
         void noArgFunction();
 
@@ -112,7 +103,7 @@ spaces.
         void noArgFunction ();
 </pre>
 
-<li>Control structures, such as if, while, do and switch - use a single space before the open paren, but no spaces inside the parentheses.
+<li>Control structures, such as if, while, do and switch &mdash; use a single space before the open paren, but no spaces inside the parentheses.
 
 </ol>
 
@@ -123,16 +114,14 @@ capitals instead of underscores for class, function and variable
 names.
 
 <li>C++ and Objective-C classes, interfaces and protocols, and other
-type names - these names should start with a capital letter and use
+type names &mdash; these names should start with a capital letter and use
 InterCaps.
 
 <pre>
     RIGHT:
-    
         class MyImportantClass
         
     WRONG:
-    
         class My_important_class
         class myImportantClass
 </pre>
@@ -142,11 +131,9 @@ start with a lowercase letter, like this:
 
 <pre>
     RIGHT:
-    
         int myInt;
         
     WRONG:
-    
         int MyInt;
         int my_int;
 </pre>
@@ -162,19 +149,13 @@ prefix of m_.
 <li>C++ member functions should follow the same naming convention as
 free functions.
 
-<li>Objective-C methods should follow the usual Cocoa naming style -
+<li>Objective-C methods should follow the usual Cocoa naming style &mdash;
 they should read like a phrase or sentence and each piece of the
 selector should start with a lowercase letter and use intercaps.
 
 <li>Objective-C instance variables should be named like local
 variables but starting with an underscore.
 
-<li>Pointer and reference types - pointer types should be written with
-a space between the type name and the * (so the * is adjacent to the
-following identifier if any). For reference types, the &amp; goes next to
-the type name.
-
-
 <li>Enum members should user InterCaps with an initial capital letter.
 
 <li>#defined constants should use all uppercase names with words
@@ -200,14 +181,40 @@ separated by underscores.
 
 <pre>
     RIGHT:
-
         urlVariable
         myURLAccessor:
 
     WRONG:
-
         uRLVariable
         myUrlAccessor:
 </pre>
 </ol>
 
+<h3>Other Punctuation</h3>
+
+<ol>
+
+<li>Pointer and reference types in C++ code &mdash; Both pointer types and reference types
+should be written with no space between the type name and the * or &amp;.
+
+<li>Pointer types in non-C++ code &mdash; Pointer types should be written with a space between the
+type and the * (so the * is adjacent to the following identifier if any).
+
+</ol>
+
+<h3>Include Statements</h3>
+
+<ol>
+
+<li>All files must #include "config.h" first.
+
+<li>All files must #include the primary header second, just after "config.h".
+So for example, Node.cpp should include Node.h first, before other files.
+This guarantees that each header's completeness is tested, to make sure it
+can be compiled without requiring any other header files be included first.
+
+<li>Other #include statements should be in sorted order (case sensitive, as
+done by the command-line sort tool or the Xcode sort selection command).
+Don't bother to organize them in a logical order.
+
+</ol>
index 4cff3db12f145dd7d5e149fb8d4cf758347a2ff8..5037e978c93c463b586406e6c9f12afe775262cb 100644 (file)
@@ -22,16 +22,15 @@ One you have checked out and built the code, you can make changes to it and then
 that contains only the differences between the current version of WebKit and your version of WebKit with your code changes.</p>
 
 <p>The <tt>svn-create-patch</tt> script, found in WebKitTools/Scripts along with the other WebKit scripts, should be used to produce patches.
-It does a <tt>svn diff</tt> operation, handling files added and removed from the repository,
-and working around path problems that normally prevent some patches created with <tt>svn diff</tt> from being applied by <tt>patch</tt>.
-Use it to create a patch as follows:</p>
+It does a <tt>svn</tt> <tt>diff</tt> operation, passing appropriate options to diff:</p>
 <p class="code">WebKitTools/Scripts/svn-create-patch > MyExcellentPatch.txt</p>
-<p>It's handy to put the <tt>WebKitTools/Scripts</tt> directory in your shell path so you can type commands like <tt>svn-create-patch</tt> without specifying the path to the script.</p>
+<p>It's handy to put the <tt>WebKitTools/Scripts</tt> directory in your shell path so you can type commands like <tt>svn-create-patch</tt>
+without specifying the path to the script.</p>
 <p>The <tt>svn-apply</tt> and <tt>svn-unapply</tt> scripts are handy for applying patches to a tree, and rolling patches out of a tree.
 They go beyond the capabilities of the <tt>patch</tt> tool by handling files added and removed from the repository.</p>
 
 <p>Once you have a patch file, it must be reviewed by one of the approved WebKit reviewers.
-To request a review, attach the patch to the bug report, and mark the patch with the flag <tt>review: ?</tt>. This will automatically
+To request a review, attach the patch to the bug report, and mark the patch with the flag <tt>review:?</tt>. This will automatically
 send mail to <a href="mailto:webkit-reviews@opendarwin.org">webkit-reviews@opendarwin.org</a> on your behalf. The 
 <a href="../quality/lifecycle.html">WebKit Bug Life Cycle</a> page
 has more information about the stages of a WebKit Bugzilla bug.</p>
@@ -40,8 +39,9 @@ has more information about the stages of a WebKit Bugzilla bug.</p>
 <a href="http://webkit.opendarwin.org/coding/coding-style.html">coding style guidelines</a>, and has received sufficient testing.
 Bug fixes should include a <a href="../quality/testing.html">new WebKit or JavaScriptCore test</a>.</p>
 
-<p>The reviewer will typically either approve the patch (by responding with an <tt>r=me</tt> in the bug report or in e-mail and marking the patch <tt>review: +</tt>)
-or request revisions to the patch (and mark the patch <tt>review: -</tt>).
+<p>The reviewer will typically either approve the patch (by responding with an <tt>r=me</tt> in the bug report or in e-mail
+and marking the patch <tt>review:+</tt>)
+or request revisions to the patch (and mark the patch <tt>review:-</tt>).
 In rare cases a patch may be permanently rejected, meaning that the reviewer believes the feature should never be committed to the tree.
 The review process can consist of multiple iterations between you and the reviewer as revisions are made to your patch.</p>
 
@@ -54,13 +54,14 @@ If no layout test can be (or needs to be) constructed for the fix, you must expl
 
 <p>In addition you must run the regression tests. The command to do that is:</p>
 <p class="code">WebKitTools/Scripts/run-webkit-tests</p>
-<p>It's handy to put the <tt>WebKitTools/Scripts</tt> directory in your shell path so you can type commands like <tt>run-webkit-tests</tt> without specifying the path to the script.</p>
+<p>It's handy to put the <tt>WebKitTools/Scripts</tt> directory in your shell path so you can type commands like
+<tt>run-webkit-tests</tt> without specifying the path to the script.</p>
 <p>Only if all layout tests pass
 (or if justification can be made for changing the expected results of the tests) will the patch be allowed in the tree.  It is the reviewer's
 responsibility to double-check that you have run the regression tests before signing off on the patch.</p>
 
 <p>If you are modifying JavaScriptCore there is an additional test suite that you need to run.  For more details on the required testing you must
-do before landing a patch, <a href="http://webkit.opendarwin.org/quality/testing.html">see this document</a>.
+do before landing a patch, see our <a href="http://webkit.opendarwin.org/quality/testing.html">testing page</a>.
 
 <p>Once a patch has been reviewed, there are two options for getting it into the tree.
 If you have check-in privileges, you can land the patch immediately once it has been reviewed.
@@ -73,11 +74,11 @@ It is your responsibility to be available should regressions arise and to respon
 <h3>Obtaining Check-In Privileges</h3>
 
 <p>Contributors with a proven track record of good patch submissions and that have demonstrated an ability to work well with the community can
-obtain check-in privileges to the WebKit source tree.  In order to obtain this check-in access, the contributor must find a reviewer who
+obtain check-in privileges to the WebKit source tree. In order to obtain this check-in access, the contributor must find a reviewer who
 will act as a sponsor.</p>
 
-<p>The contributor must then sign and submit the contributor agreement (link forthcoming) to Apple in order to be granted check-in
-access.</p>
+<p>The sponsor arranges a copy of the committer agreement to be sent to the contributor.
+Once the contributor sends a copy of the signed agreement to Apple, he receives check-in access.</p>
 
 <h3>Becoming a Reviewer</h3>
 
@@ -85,8 +86,8 @@ access.</p>
 contributor is effectively functioning as an expert in a particular area of the code and is qualified to review patches submitted in
 that area.</p>
 
-<p>Reviewers are always responsible only for areas of code in which they are knowledgeable.  If the reviewer does not feel qualified to handle
-a particular patch, then he will defer to another reviewer.</p>
+<p>Reviewers are always responsible only for areas of code in which they are knowledgeable.
+If the reviewer does not feel qualified to handle a particular patch, then he will defer to another reviewer.</p>
 
 </div>
 </body>