Skip to content

G
git2
Commit 1631147c by Russell Belfer
Options

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.
parent 01be7863
Showing with 174 additions and 63 deletions
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").
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.
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.
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. 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.
Other licenses have other requirements; check the license of the library you're
porting code *from* to see what you need to do.
## Style Guide
## Styleguide
`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
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.
CONVENTIONS.md
# 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);
```
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).
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment