Updates to CONTRIBUTING and CONVENTIONS The discussion about converting some of our foreach-style APIs to use iterator objects got me wanting to make a list of good starter projects. I put it in CONTRIBUTING.md and then went crazy with updates to that file and to CONVENTIONS.md.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 8561794..04a537f 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -7,41 +7,94 @@ your help.
We hang out in the #libgit2 channel on irc.freenode.net.
+Also, feel free to open an
+[Issue](https://github.com/libgit2/libgit2/issues/new) to start a discussion
+about any concerns you have. We like to use Issues for that so there is an
+easily accessible permanent record of the conversation.
+
## Reporting Bugs
-First, know which version of libgit2 your problem is in. Compile and test
-against the `development` branch to avoid re-reporting an issue that's already
-been fixed.
+First, know which version of libgit2 your problem is in and include it in
+your bug report. This can either be a tag (e.g.
+[v0.17.0](https://github.com/libgit2/libgit2/tree/v0.17.0) ) or a commit
+SHA (e.g.
+[01be7863](https://github.com/libgit2/libgit2/commit/01be786319238fd6507a08316d1c265c1a89407f)
+). Using [`git describe`](http://git-scm.com/docs/git-describe) is a great
+way to tell us what version you're working with.
+
+If you're not running against the latest `development` branch version,
+please compile and test against that to avoid re-reporting an issue that's
+already been fixed.
-It's *incredibly* helpful to be able to reproduce the problem. Please include
-a bit of code and/or a zipped repository (if possible). Note that some of the
-developers are employees of GitHub, so if your repository is private, find us
-on IRC and we'll figure out a way to help you.
+It's *incredibly* helpful to be able to reproduce the problem. Please
+include a list of steps, a bit of code, and/or a zipped repository (if
+possible). Note that some of the libgit2 developers are employees of
+GitHub, so if your repository is private, find us on IRC and we'll figure
+out a way to help you.
## Pull Requests
-Life will be a lot easier for you if you create a named branch for your
-contribution, rather than just using your fork's `development`.
+Our work flow is a typical GitHub flow, where contributors fork the
+[libgit2 repository](https://github.com/libgit2/libgit2), make their changes
+on branch, and submit a
+[Pull Request](https://help.github.com/articles/using-pull-requests)
+(a.k.a. "PR").
-It's helpful if you include a nice description of your change with your PR; if
-someone has to read the whole diff to figure out why you're contributing in the
-first place, you're less likely to get feedback and have your change merged in.
+Life will be a lot easier for you (and us) if you follow this pattern
+(i.e. fork, named branch, submit PR). If you use your fork's `development`
+branch, things can get messy.
+
+Please include a nice description of your changes with your PR; if we have
+to read the whole diff to figure out why you're contributing in the first
+place, you're less likely to get feedback and have your change merged in.
## Porting Code From Other Open-Source Projects
-The most common case here is porting code from core Git. Git is a GPL project,
-which means that in order to port code to this project, we need the explicit
-permission of the author. Check the
+`libgit2` is licensed under the terms of the GPL v2 with a linking
+exception. Any code brought in must be compatible with those terms.
+
+The most common case is porting code from core Git. Git is a pure GPL
+project, which means that in order to port code to this project, we need the
+explicit permission of the author. Check the
[`git.git-authors`](https://github.com/libgit2/libgit2/blob/development/git.git-authors)
-file for authors who have already consented; feel free to add someone if you've
-obtained their consent.
+file for authors who have already consented; feel free to add someone if
+you've obtained their consent.
-Other licenses have other requirements; check the license of the library you're
-porting code *from* to see what you need to do.
+Other licenses have other requirements; check the license of the library
+you're porting code *from* to see what you need to do. As a general rule,
+MIT and BSD (3-clause) licenses are typically no problem. Apache 2.0
+license typically doesn't work due to GPL incompatibility.
-## Styleguide
+## Style Guide
-We like to keep the source code consistent and easy to read. Maintaining this
-takes some discipline, but it's been more than worth it. Take a look at the
+`libgit2` is written in [ANSI C](http://en.wikipedia.org/wiki/ANSI_C)
+(a.k.a. C89) with some specific conventions for function and type naming,
+code formatting, and testing.
+
+We like to keep the source code consistent and easy to read. Maintaining
+this takes some discipline, but it's been more than worth it. Take a look
+at the
[conventions file](https://github.com/libgit2/libgit2/blob/development/CONVENTIONS.md).
+## Starter Projects
+
+So, you want to start helping out with `libgit2`? That's fantastic? We
+welcome contributions and we promise we'll try to be nice.
+
+If you want to jump in, you can look at our issues list to see if there
+are any unresolved issues to jump in on. Also, here is a list of some
+smaller project ideas that could help you become familiar with the code
+base and make a nice first step:
+
+* Convert a `git_*modulename*_foreach()` callback-based iteration API
+ into a `git_*modulename*_iterator` object with a create/advance style
+ of API. This helps folks writing language bindings and usually isn't
+ too complicated.
+* Write a new `examples/` program that mirrors a particular core git
+ command. (See `examples/diff.c` for example.) This lets you (and us)
+ easily exercise a particular facet of the API and measure compatability
+ and feature parity with core git.
+* Submit a PR to clarify documentation! While we do try to document all of
+ the APIs, your fresh eyes on the documentation will find areas that are
+ confusing much more easily.
+
diff --git a/CONVENTIONS.md b/CONVENTIONS.md
index 1e909a3..67b60f4 100644
--- a/CONVENTIONS.md
+++ b/CONVENTIONS.md
@@ -1,12 +1,39 @@
# Libgit2 Conventions
-We like to keep the source consistent and readable. Herein are some guidelines
-that should help with that.
+We like to keep the source consistent and readable. Herein are some
+guidelines that should help with that.
+## Compatibility
+
+`libgit2` runs on many different platforms with many different compilers.
+It is written in [ANSI C](http://en.wikipedia.org/wiki/ANSI_C) (a.k.a. C89)
+with some specific standards for function and type naming, code formatting,
+and testing.
+
+We try to avoid more recent extensions to maximize portability. We also, to
+the greatest extent possible, try to avoid lots of `#ifdef`s inside the core
+code base. This is somewhat unavoidable, but since it can really hamper
+maintainability, we keep it to a minimum.
+
+## Match Surrounding Code
+
+If there is one rule to take away from this document, it is *new code should
+match the surrounding code in a way that makes it impossible to distinguish
+the new from the old.* Consistency is more important to us than anyone's
+personal opinion about where braces should be placed or spaces vs. tabs.
+
+If a section of code is being completely rewritten, it is okay to bring it
+in line with the standards that are laid out here, but we will not accept
+submissions that contain a large number of changes that are merely
+reformatting.
## Naming Things
-All types and functions start with `git_`, and all #define macros start with `GIT_`.
+All external types and functions start with `git_` and all `#define` macros
+start with `GIT_`. The `libgit2` API is mostly broken into related
+functional modules each with a corresponding header. All functions in a
+module should be named like `git_modulename_functioname()`
+(e.g. `git_repository_open()`).
Functions with a single output parameter should name that parameter `out`.
Multiple outputs should be named `foo_out`, `bar_out`, etc.
@@ -14,26 +41,27 @@ Multiple outputs should be named `foo_out`, `bar_out`, etc.
Parameters of type `git_oid` should be named `id`, or `foo_id`. Calls that
return an OID should be named `git_foo_id`.
-Where there is a callback passed in, the `void *` that is passed into it should
-be named "payload".
+Where a callback function is used, the function should also include a
+user-supplied extra input that is a `void *` named "payload" that will be
+passed through to the callback at each invocation.
-## Typedef
+## Typedefs
-Wherever possible, use `typedef`. If a structure is just a collection of
-function pointers, the pointer types don't need to be separately typedef'd, but
-loose function pointer types should be.
+Wherever possible, use `typedef`. In some cases, if a structure is just a
+collection of function pointers, the pointer types don't need to be
+separately typedef'd, but loose function pointer types should be.
## Exports
All exported functions must be declared as:
-```C
+```c
GIT_EXTERN(result_type) git_modulename_functionname(arg_list);
```
## Internals
-Functions whose modulename is followed by two underscores,
+Functions whose *modulename* is followed by two underscores,
for example `git_odb__read_packed`, are semi-private functions.
They are primarily intended for use within the library itself,
and may disappear or change their signature in a future release.
@@ -43,42 +71,46 @@ and may disappear or change their signature in a future release.
Out parameters come first.
Whenever possible, pass argument pointers as `const`. Some structures (such
-as `git_repository` and `git_index`) have internal structure that prevents
-this.
+as `git_repository` and `git_index`) have mutable internal structure that
+prevents this.
Callbacks should always take a `void *` payload as their last parameter.
-Callback pointers are grouped with their payloads, and come last when passed as
-arguments:
+Callback pointers are grouped with their payloads, and typically come last
+when passed as arguments:
-```C
-int foo(git_repository *repo, git_foo_cb callback, void *payload);
+```c
+int git_foo(git_repository *repo, git_foo_cb callback, void *payload);
```
-
## Memory Ownership
Some APIs allocate memory which the caller is responsible for freeing; others
return a pointer into a buffer that's owned by some other object. Make this
explicit in the documentation.
-
## Return codes
-Return an `int` when a public API can fail in multiple ways. These may be
-transformed into exception types in some bindings, so returning a semantically
-appropriate error code is important. Check
-[`errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h)
+Most public APIs should return an `int` error code. As is typical with most
+C library functions, a zero value indicates success and a negative value
+indicates failure.
+
+Some bindings will transform these returned error codes into exception
+types, so returning a semantically appropriate error code is important.
+Check
+[`include/git2/errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h)
for the return codes already defined.
-Use `giterr_set` to provide extended error information to callers.
+In your implementation, use `giterr_set()` to provide extended error
+information to callers.
-If an error is not to be propagated, use `giterr_clear` to prevent callers from
-getting the wrong error message later on.
+If a `libgit2` function internally invokes another function that reports an
+error, but the error is not propagated up, use `giterr_clear()` to prevent
+callers from getting the wrong error message later on.
-## Opaque Structs
+## Structs
-Most types should be opaque, e.g.:
+Most public types should be opaque, e.g.:
```C
typedef struct git_odb git_odb;
@@ -95,10 +127,10 @@ append to the end of the structure.
## Option Structures
-If a function's parameter count is too high, it may be desirable to package up
-the options in a structure. Make them transparent, include a version field,
-and provide an initializer constant or constructor. Using these structures
-should be this easy:
+If a function's parameter count is too high, it may be desirable to package
+up the options in a structure. Make them transparent, include a version
+field, and provide an initializer constant or constructor. Using these
+structures should be this easy:
```C
git_foo_options opts = GIT_FOO_OPTIONS_INIT;
@@ -108,30 +140,40 @@ git_foo(&opts);
## Enumerations
-Typedef all enumerated types. If each option stands alone, use the enum type
-for passing them as parameters; if they are flags, pass them as `unsigned int`.
+Typedef all enumerated types. If each option stands alone, use the enum
+type for passing them as parameters; if they are flags to be OR'ed together,
+pass them as `unsigned int` or `uint32_t` or some appropriate type.
## Code Layout
-Try to keep lines less than 80 characters long. Use common sense to wrap most
-code lines; public function declarations should use this convention:
+Try to keep lines less than 80 characters long. This is a loose
+requirement, but going significantly over 80 columns is not nice.
-```C
+Use common sense to wrap most code lines; public function declarations
+can use a couple of different styles:
+
+```c
+/** All on one line is okay if it fits */
+GIT_EXTERN(int) git_foo_simple(git_oid *id);
+
+/** Otherwise one argument per line is a good next step */
GIT_EXTERN(int) git_foo_id(
- git_oid **out,
- int a,
- int b);
+ git_oid **out,
+ int a,
+ int b);
```
-Indentation is done with tabs; set your editor's tab width to 3 for best effect.
+Indent with tabs; set your editor's tab width to 4 for best effect.
+Avoid trailing whitespace and only commit Unix-style newlines (i.e. no CRLF
+in the repository - just set `core.autocrlf` to true if you are writing code
+on a Windows machine).
## Documentation
All comments should conform to Doxygen "javadoc" style conventions for
-formatting the public API documentation. Try to document every parameter, and
-keep the comments up to date if you change the parameter list.
-
+formatting the public API documentation. Try to document every parameter,
+and keep the comments up to date if you change the parameter list.
## Public Header Template
@@ -167,3 +209,19 @@ All inlined functions must be declared as:
GIT_INLINE(result_type) git_modulename_functionname(arg_list);
```
+## Tests
+
+`libgit2` uses the (clar)[https://github.com/vmg/clar] testing framework.
+
+All PRs should have corresponding tests.
+
+* If the PR fixes an existing issue, the test should fail prior to applying
+ the PR and succeed after applying it.
+* If the PR is for new functionality, then the tests should exercise that
+ new functionality to a certain extent. We don't require 100% coverage
+ right now (although we are getting stricter over time).
+
+When adding new tests, we prefer if you attempt to reuse existing test data
+(in `tests-clar/resources/`) if possible. If you are going to add new test
+repositories, please try to strip them of unnecessary files (e.g. sample
+hooks, etc).