many updates for HACKING.md
authorEvan Martin <martine@danga.com>
Thu, 16 Aug 2012 18:25:55 +0000 (11:25 -0700)
committerEvan Martin <martine@danga.com>
Thu, 16 Aug 2012 18:25:55 +0000 (11:25 -0700)
In particular, describe a policy for good patches.

HACKING.md

index 8a477f3..bc23773 100644 (file)
@@ -1,11 +1,59 @@
+## Basic overview
+
+`./configure.py` generates the `build.ninja` files used to build
+ninja.  It accepts various flags to adjust build parameters.
+
+The primary build target of interest is `ninja`, but when hacking on
+Ninja your changes should be testable so it's more useful to build
+and run `ninja_test` when developing.
+
+(`./bootstrap.py` creates a bootstrap `ninja` and runs the above
+process; it's only necessary to run if you don't have a copy of
+`ninja` to build with.)
 
 ### Adjusting build flags
 
-    CFLAGS=-O3 ./configure.py
+Build in "debug" mode while developing (disables optimizations and builds
+way faster on Windows):
+
+    ./configure.py --debug
+
+To use clang, set `CXX`:
+
+    CXX=clang++ ./configure.py
+
+## How to successfully make changes to Ninja
+
+Github pull requests are convenient for me to merge (I can just click
+a button and it's all handled server-side), but I'm also comfortable
+accepting pre-github git patches (via `send-email` etc.).
+
+Good pull requests have all of these attributes:
+
+* Are scoped to one specific issue
+* Include a test to demonstrate their correctness
+* Update the docs where relevant
+* Match the Ninja coding style (see below)
+* Don't include a mess of "oops, fix typo" commits
 
-### Testing
+These are typically merged without hesitation.  If a change is lacking
+any of the above I usually will ask you to fix it, though there are
+obvious exceptions (fixing typos in comments don't need tests).
 
-#### Installing gtest
+I am very wary of changes that increase the complexity of Ninja (in
+particular, new build file syntax or command-line flags) or increase
+the maintenance burden of Ninja.  Ninja is already successfully in use
+by hundreds of developers for large projects and it already achieves
+(most of) the goals I set out for it to do.  It's probably best to
+discuss new feature ideas on the mailing list before I shoot down your
+patch.
+
+## Testing
+
+### Installing gtest
+
+The `ninja_test` binary, containing all the tests, depends on the
+googletest (gtest) library.
 
 * On older Ubuntus it'll install as libraries into `/usr/lib`:
 
         apt-get install libgtest-dev
         ./configure --with-gtest=/usr/src/gtest
 
-* Otherwise you need to download it, unpack it, and pass --with-gtest
-  as appropriate.
+* Otherwise you need to download it, unpack it, and pass
+  `--with-gtest` to `configure.py`.  Get it from [its downloads
+  page](http://code.google.com/p/googletest/downloads/list).
 
-#### Test-driven development
+### Test-driven development
 
 Set your build command to
 
     ./ninja ninja_test && ./ninja_test --gtest_filter=MyTest.Name
 
-now you can repeatedly run that while developing until the tests pass.
-Remember to build "all" before committing to verify the other source
-still works!
+now you can repeatedly run that while developing until the tests pass
+(I frequently set it as my compilation command in Emacs).  Remember to
+build "all" before committing to verify the other source still works!
 
-### Testing performance impact of changes
+## Testing performance impact of changes
 
-If you have a Chrome build handy, it's a good test case.
-Otherwise, https://github.com/martine/ninja/downloads has a copy of
-the Chrome build files (and depfiles). You can untar that, then run
+If you have a Chrome build handy, it's a good test case.  Otherwise,
+[the github downoads page](https://github.com/martine/ninja/downloads)
+has a copy of the Chrome build files (and depfiles). You can untar
+that, then run
 
     path/to/my/ninja chrome
 
@@ -55,7 +105,8 @@ Generally it's the [Google C++ coding style][], but in brief:
 * Member methods are camelcase, expect for trivial getters which are
   underscore separated.
 * Local variables are underscore separated.
-* Member variables are underscore separated and suffixed by an extra underscore.
+* Member variables are underscore separated and suffixed by an extra
+  underscore.
 * Two spaces indentation.
 * Opening braces is at the end of line.
 * Lines are 80 columns maximum.
@@ -78,7 +129,12 @@ Generally it's the [Google C++ coding style][], but in brief:
     sudo apt-get install asciidoc --no-install-recommends
     ./ninja manual
 
-## Building on Windows
+### Building the code documentation
+
+    sudo apt-get install doxygen
+    ./ninja doxygen
+
+## Building for Windows
 
 While developing, it's helpful to copy `ninja.exe` to another name like
 `n.exe`; otherwise, rebuilds will be unable to write `ninja.exe` because
@@ -92,6 +148,13 @@ it's locked while in use.
 
 [Python for Windows]: http://www.python.org/getit/windows/
 
+### Via mingw on Windows (not well supported)
+
+* Install mingw, msys, and python
+* In the mingw shell, put Python in your path, and `python bootstrap.py`
+* To reconfigure, run `python configure.py`
+* Remember to strip the resulting executable if size matters to you
+
 ### Via mingw on Linux (not well supported)
 
 * `sudo apt-get install gcc-mingw32 wine`
@@ -100,14 +163,3 @@ it's locked while in use.
 * Build `ninja.exe` using a Linux ninja binary: `/path/to/linux/ninja`
 * Run: `./ninja.exe`  (implicitly runs through wine(!))
 
-### Via mingw on Windows (not well supported)
-* Install mingw, msys, and python
-* In the mingw shell, put Python in your path, and: python bootstrap.py
-* To reconfigure, run `python configure.py`
-* Remember to strip the resulting executable if size matters to you
-
-## Clang
-
-To use clang, set `CXX`:
-
-    CXX=clang++ ./configure.py