Skip to content

Commit bb19532

Browse files
author
Vicent Martí
committed
Merge pull request libgit2#1386 from arrbee/update-docs
Update contributing and conventions
2 parents 01be786 + a313de0 commit bb19532

File tree

3 files changed

+177
-86
lines changed

3 files changed

+177
-86
lines changed

CONTRIBUTING.md

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,41 +7,93 @@ your help.
77

88
We hang out in the #libgit2 channel on irc.freenode.net.
99

10+
Also, feel free to open an
11+
[Issue](https://github.com/libgit2/libgit2/issues/new) to start a discussion
12+
about any concerns you have. We like to use Issues for that so there is an
13+
easily accessible permanent record of the conversation.
14+
1015
## Reporting Bugs
1116

12-
First, know which version of libgit2 your problem is in. Compile and test
13-
against the `development` branch to avoid re-reporting an issue that's already
14-
been fixed.
17+
First, know which version of libgit2 your problem is in and include it in
18+
your bug report. This can either be a tag (e.g.
19+
[v0.17.0](https://github.com/libgit2/libgit2/tree/v0.17.0) ) or a commit
20+
SHA (e.g.
21+
[01be7863](https://github.com/libgit2/libgit2/commit/01be786319238fd6507a08316d1c265c1a89407f)
22+
). Using [`git describe`](http://git-scm.com/docs/git-describe) is a great
23+
way to tell us what version you're working with.
24+
25+
If you're not running against the latest `development` branch version,
26+
please compile and test against that to avoid re-reporting an issue that's
27+
already been fixed.
1528

16-
It's *incredibly* helpful to be able to reproduce the problem. Please include
17-
a bit of code and/or a zipped repository (if possible). Note that some of the
18-
developers are employees of GitHub, so if your repository is private, find us
19-
on IRC and we'll figure out a way to help you.
29+
It's *incredibly* helpful to be able to reproduce the problem. Please
30+
include a list of steps, a bit of code, and/or a zipped repository (if
31+
possible). Note that some of the libgit2 developers are employees of
32+
GitHub, so if your repository is private, find us on IRC and we'll figure
33+
out a way to help you.
2034

2135
## Pull Requests
2236

23-
Life will be a lot easier for you if you create a named branch for your
24-
contribution, rather than just using your fork's `development`.
37+
Our work flow is a typical GitHub flow, where contributors fork the
38+
[libgit2 repository](https://github.com/libgit2/libgit2), make their changes
39+
on branch, and submit a
40+
[Pull Request](https://help.github.com/articles/using-pull-requests)
41+
(a.k.a. "PR").
2542

26-
It's helpful if you include a nice description of your change with your PR; if
27-
someone has to read the whole diff to figure out why you're contributing in the
28-
first place, you're less likely to get feedback and have your change merged in.
43+
Life will be a lot easier for you (and us) if you follow this pattern
44+
(i.e. fork, named branch, submit PR). If you use your fork's `development`
45+
branch, things can get messy.
46+
47+
Please include a nice description of your changes with your PR; if we have
48+
to read the whole diff to figure out why you're contributing in the first
49+
place, you're less likely to get feedback and have your change merged in.
2950

3051
## Porting Code From Other Open-Source Projects
3152

32-
The most common case here is porting code from core Git. Git is a GPL project,
33-
which means that in order to port code to this project, we need the explicit
34-
permission of the author. Check the
53+
`libgit2` is licensed under the terms of the GPL v2 with a linking
54+
exception. Any code brought in must be compatible with those terms.
55+
56+
The most common case is porting code from core Git. Git is a pure GPL
57+
project, which means that in order to port code to this project, we need the
58+
explicit permission of the author. Check the
3559
[`git.git-authors`](https://github.com/libgit2/libgit2/blob/development/git.git-authors)
36-
file for authors who have already consented; feel free to add someone if you've
37-
obtained their consent.
60+
file for authors who have already consented; feel free to add someone if
61+
you've obtained their consent.
3862

39-
Other licenses have other requirements; check the license of the library you're
40-
porting code *from* to see what you need to do.
63+
Other licenses have other requirements; check the license of the library
64+
you're porting code *from* to see what you need to do. As a general rule,
65+
MIT and BSD (3-clause) licenses are typically no problem. Apache 2.0
66+
license typically doesn't work due to GPL incompatibility.
4167

42-
## Styleguide
68+
## Style Guide
4369

44-
We like to keep the source code consistent and easy to read. Maintaining this
45-
takes some discipline, but it's been more than worth it. Take a look at the
70+
`libgit2` is written in [ANSI C](http://en.wikipedia.org/wiki/ANSI_C)
71+
(a.k.a. C89) with some specific conventions for function and type naming,
72+
code formatting, and testing.
73+
74+
We like to keep the source code consistent and easy to read. Maintaining
75+
this takes some discipline, but it's been more than worth it. Take a look
76+
at the
4677
[conventions file](https://github.com/libgit2/libgit2/blob/development/CONVENTIONS.md).
4778

79+
## Starter Projects
80+
81+
So, you want to start helping out with `libgit2`? That's fantastic? We
82+
welcome contributions and we promise we'll try to be nice.
83+
84+
If you want to jump in, you can look at our issues list to see if there
85+
are any unresolved issues to jump in on. Also, here is a list of some
86+
smaller project ideas that could help you become familiar with the code
87+
base and make a nice first step:
88+
89+
* Convert a `git_*modulename*_foreach()` callback-based iteration API
90+
into a `git_*modulename*_iterator` object with a create/advance style
91+
of API. This helps folks writing language bindings and usually isn't
92+
too complicated.
93+
* Write a new `examples/` program that mirrors a particular core git
94+
command. (See `examples/diff.c` for example.) This lets you (and us)
95+
easily exercise a particular facet of the API and measure compatability
96+
and feature parity with core git.
97+
* Submit a PR to clarify documentation! While we do try to document all of
98+
the APIs, your fresh eyes on the documentation will find areas that are
99+
confusing much more easily.

CONVENTIONS.md

Lines changed: 102 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,67 @@
11
# Libgit2 Conventions
22

3-
We like to keep the source consistent and readable. Herein are some guidelines
4-
that should help with that.
3+
We like to keep the source consistent and readable. Herein are some
4+
guidelines that should help with that.
55

6+
## Compatibility
7+
8+
`libgit2` runs on many different platforms with many different compilers.
9+
It is written in [ANSI C](http://en.wikipedia.org/wiki/ANSI_C) (a.k.a. C89)
10+
with some specific standards for function and type naming, code formatting,
11+
and testing.
12+
13+
We try to avoid more recent extensions to maximize portability. We also, to
14+
the greatest extent possible, try to avoid lots of `#ifdef`s inside the core
15+
code base. This is somewhat unavoidable, but since it can really hamper
16+
maintainability, we keep it to a minimum.
17+
18+
## Match Surrounding Code
19+
20+
If there is one rule to take away from this document, it is *new code should
21+
match the surrounding code in a way that makes it impossible to distinguish
22+
the new from the old.* Consistency is more important to us than anyone's
23+
personal opinion about where braces should be placed or spaces vs. tabs.
24+
25+
If a section of code is being completely rewritten, it is okay to bring it
26+
in line with the standards that are laid out here, but we will not accept
27+
submissions that contain a large number of changes that are merely
28+
reformatting.
629

730
## Naming Things
831

9-
All types and functions start with `git_`, and all #define macros start with `GIT_`.
32+
All external types and functions start with `git_` and all `#define` macros
33+
start with `GIT_`. The `libgit2` API is mostly broken into related
34+
functional modules each with a corresponding header. All functions in a
35+
module should be named like `git_modulename_functioname()`
36+
(e.g. `git_repository_open()`).
1037

1138
Functions with a single output parameter should name that parameter `out`.
1239
Multiple outputs should be named `foo_out`, `bar_out`, etc.
1340

1441
Parameters of type `git_oid` should be named `id`, or `foo_id`. Calls that
1542
return an OID should be named `git_foo_id`.
1643

17-
Where there is a callback passed in, the `void *` that is passed into it should
18-
be named "payload".
44+
Where a callback function is used, the function should also include a
45+
user-supplied extra input that is a `void *` named "payload" that will be
46+
passed through to the callback at each invocation.
1947

20-
## Typedef
48+
## Typedefs
2149

22-
Wherever possible, use `typedef`. If a structure is just a collection of
23-
function pointers, the pointer types don't need to be separately typedef'd, but
24-
loose function pointer types should be.
50+
Wherever possible, use `typedef`. In some cases, if a structure is just a
51+
collection of function pointers, the pointer types don't need to be
52+
separately typedef'd, but loose function pointer types should be.
2553

2654
## Exports
2755

2856
All exported functions must be declared as:
2957

30-
```C
58+
```c
3159
GIT_EXTERN(result_type) git_modulename_functionname(arg_list);
3260
```
3361
3462
## Internals
3563
36-
Functions whose modulename is followed by two underscores,
64+
Functions whose *modulename* is followed by two underscores,
3765
for example `git_odb__read_packed`, are semi-private functions.
3866
They are primarily intended for use within the library itself,
3967
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.
4371
Out parameters come first.
4472
4573
Whenever possible, pass argument pointers as `const`. Some structures (such
46-
as `git_repository` and `git_index`) have internal structure that prevents
47-
this.
74+
as `git_repository` and `git_index`) have mutable internal structure that
75+
prevents this.
4876
4977
Callbacks should always take a `void *` payload as their last parameter.
50-
Callback pointers are grouped with their payloads, and come last when passed as
51-
arguments:
78+
Callback pointers are grouped with their payloads, and typically come last
79+
when passed as arguments:
5280
53-
```C
54-
int foo(git_repository *repo, git_foo_cb callback, void *payload);
81+
```c
82+
int git_foo(git_repository *repo, git_foo_cb callback, void *payload);
5583
```
5684

57-
5885
## Memory Ownership
5986

6087
Some APIs allocate memory which the caller is responsible for freeing; others
6188
return a pointer into a buffer that's owned by some other object. Make this
6289
explicit in the documentation.
6390

64-
6591
## Return codes
6692

67-
Return an `int` when a public API can fail in multiple ways. These may be
68-
transformed into exception types in some bindings, so returning a semantically
69-
appropriate error code is important. Check
70-
[`errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h)
93+
Most public APIs should return an `int` error code. As is typical with most
94+
C library functions, a zero value indicates success and a negative value
95+
indicates failure.
96+
97+
Some bindings will transform these returned error codes into exception
98+
types, so returning a semantically appropriate error code is important.
99+
Check
100+
[`include/git2/errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h)
71101
for the return codes already defined.
72102

73-
Use `giterr_set` to provide extended error information to callers.
103+
In your implementation, use `giterr_set()` to provide extended error
104+
information to callers.
74105

75-
If an error is not to be propagated, use `giterr_clear` to prevent callers from
76-
getting the wrong error message later on.
106+
If a `libgit2` function internally invokes another function that reports an
107+
error, but the error is not propagated up, use `giterr_clear()` to prevent
108+
callers from getting the wrong error message later on.
77109

78110

79-
## Opaque Structs
111+
## Structs
80112

81-
Most types should be opaque, e.g.:
113+
Most public types should be opaque, e.g.:
82114

83115
```C
84116
typedef struct git_odb git_odb;
@@ -95,10 +127,10 @@ append to the end of the structure.
95127

96128
## Option Structures
97129

98-
If a function's parameter count is too high, it may be desirable to package up
99-
the options in a structure. Make them transparent, include a version field,
100-
and provide an initializer constant or constructor. Using these structures
101-
should be this easy:
130+
If a function's parameter count is too high, it may be desirable to package
131+
up the options in a structure. Make them transparent, include a version
132+
field, and provide an initializer constant or constructor. Using these
133+
structures should be this easy:
102134

103135
```C
104136
git_foo_options opts = GIT_FOO_OPTIONS_INIT;
@@ -108,30 +140,40 @@ git_foo(&opts);
108140
109141
## Enumerations
110142
111-
Typedef all enumerated types. If each option stands alone, use the enum type
112-
for passing them as parameters; if they are flags, pass them as `unsigned int`.
143+
Typedef all enumerated types. If each option stands alone, use the enum
144+
type for passing them as parameters; if they are flags to be OR'ed together,
145+
pass them as `unsigned int` or `uint32_t` or some appropriate type.
113146
114147
## Code Layout
115148
116-
Try to keep lines less than 80 characters long. Use common sense to wrap most
117-
code lines; public function declarations should use this convention:
149+
Try to keep lines less than 80 characters long. This is a loose
150+
requirement, but going significantly over 80 columns is not nice.
118151
119-
```C
152+
Use common sense to wrap most code lines; public function declarations
153+
can use a couple of different styles:
154+
155+
```c
156+
/** All on one line is okay if it fits */
157+
GIT_EXTERN(int) git_foo_simple(git_oid *id);
158+
159+
/** Otherwise one argument per line is a good next step */
120160
GIT_EXTERN(int) git_foo_id(
121-
git_oid **out,
122-
int a,
123-
int b);
161+
git_oid **out,
162+
int a,
163+
int b);
124164
```
125165

126-
Indentation is done with tabs; set your editor's tab width to 3 for best effect.
166+
Indent with tabs; set your editor's tab width to 4 for best effect.
127167

168+
Avoid trailing whitespace and only commit Unix-style newlines (i.e. no CRLF
169+
in the repository - just set `core.autocrlf` to true if you are writing code
170+
on a Windows machine).
128171

129172
## Documentation
130173

131174
All comments should conform to Doxygen "javadoc" style conventions for
132-
formatting the public API documentation. Try to document every parameter, and
133-
keep the comments up to date if you change the parameter list.
134-
175+
formatting the public API documentation. Try to document every parameter,
176+
and keep the comments up to date if you change the parameter list.
135177

136178
## Public Header Template
137179

@@ -167,3 +209,19 @@ All inlined functions must be declared as:
167209
GIT_INLINE(result_type) git_modulename_functionname(arg_list);
168210
```
169211
212+
## Tests
213+
214+
`libgit2` uses the [clar](https://github.com/vmg/clar) testing framework.
215+
216+
All PRs should have corresponding tests.
217+
218+
* If the PR fixes an existing issue, the test should fail prior to applying
219+
the PR and succeed after applying it.
220+
* If the PR is for new functionality, then the tests should exercise that
221+
new functionality to a certain extent. We don't require 100% coverage
222+
right now (although we are getting stricter over time).
223+
224+
When adding new tests, we prefer if you attempt to reuse existing test data
225+
(in `tests-clar/resources/`) if possible. If you are going to add new test
226+
repositories, please try to strip them of unnecessary files (e.g. sample
227+
hooks, etc).

examples/diff.c

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,9 @@ static int resolve_to_tree(
1515
git_repository *repo, const char *identifier, git_tree **tree)
1616
{
1717
int err = 0;
18-
size_t len = strlen(identifier);
19-
git_oid oid;
2018
git_object *obj = NULL;
2119

22-
/* try to resolve as OID */
23-
if (git_oid_fromstrn(&oid, identifier, len) == 0)
24-
git_object_lookup_prefix(&obj, repo, &oid, len, GIT_OBJ_ANY);
25-
26-
/* try to resolve as reference */
27-
if (obj == NULL) {
28-
git_reference *ref, *resolved;
29-
if (git_reference_lookup(&ref, repo, identifier) == 0) {
30-
git_reference_resolve(&resolved, ref);
31-
git_reference_free(ref);
32-
if (resolved) {
33-
git_object_lookup(&obj, repo, git_reference_target(resolved), GIT_OBJ_ANY);
34-
git_reference_free(resolved);
35-
}
36-
}
37-
}
38-
39-
if (obj == NULL)
20+
if (git_revparse_single(&obj, repo, identifier) < 0)
4021
return GIT_ENOTFOUND;
4122

4223
switch (git_object_type(obj)) {

0 commit comments

Comments
 (0)