diff options
| -rw-r--r-- | CONTRIBUTING.md | 96 | ||||
| -rw-r--r-- | CONVENTIONS.md | 146 | ||||
| -rw-r--r-- | examples/diff.c | 21 |
3 files changed, 177 insertions, 86 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 856179481..28ef27f42 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,41 +7,93 @@ 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 1e909a3e8..59b41a2e6 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). diff --git a/examples/diff.c b/examples/diff.c index 17bf8427e..a153b493b 100644 --- a/examples/diff.c +++ b/examples/diff.c @@ -15,28 +15,9 @@ static int resolve_to_tree( git_repository *repo, const char *identifier, git_tree **tree) { int err = 0; - size_t len = strlen(identifier); - git_oid oid; git_object *obj = NULL; - /* try to resolve as OID */ - if (git_oid_fromstrn(&oid, identifier, len) == 0) - git_object_lookup_prefix(&obj, repo, &oid, len, GIT_OBJ_ANY); - - /* try to resolve as reference */ - if (obj == NULL) { - git_reference *ref, *resolved; - if (git_reference_lookup(&ref, repo, identifier) == 0) { - git_reference_resolve(&resolved, ref); - git_reference_free(ref); - if (resolved) { - git_object_lookup(&obj, repo, git_reference_target(resolved), GIT_OBJ_ANY); - git_reference_free(resolved); - } - } - } - - if (obj == NULL) + if (git_revparse_single(&obj, repo, identifier) < 0) return GIT_ENOTFOUND; switch (git_object_type(obj)) { |