From 41b00cccb14ad1ffae703fe0f7f5843a5ec7b5b0 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 20 Nov 2012 20:59:58 -0700 Subject: [PATCH 1/3] Add contributing guidelines --- CONTRIBUTING.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..488732a62 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,47 @@ +# Welcome to libgit2! + +We're making it easy to do interesting things with git, and we'd love to have +your help. + +## Discussion & Chat + +We hang out in the #libgit2 channel on irc.freenode.net. + +## 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. + +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. + +## 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`. + +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. + +## 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 +[`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. + +Other licenses have other requirements; check the license of the library you're +porting code *from* to see what you need to do. + +## Styleguide + +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). + From ee72ffd060aa45e3ce0c8865145f99f96d00a563 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 20 Nov 2012 21:04:52 -0700 Subject: [PATCH 2/3] Markdownize CONVENTIONS --- CONTRIBUTING.md | 2 +- CONVENTIONS => CONVENTIONS.md | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) rename CONVENTIONS => CONVENTIONS.md (98%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 488732a62..856179481 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,5 +43,5 @@ porting code *from* to see what you need to do. 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). +[conventions file](https://github.com/libgit2/libgit2/blob/development/CONVENTIONS.md). diff --git a/CONVENTIONS b/CONVENTIONS.md similarity index 98% rename from CONVENTIONS rename to CONVENTIONS.md index f082d8e6c..06d3e875d 100644 --- a/CONVENTIONS +++ b/CONVENTIONS.md @@ -14,9 +14,9 @@ Type Definitions Most types should be opaque, e.g.: ----- +```C typedef struct git_odb git_odb; ----- +``` with allocation functions returning an "instance" created within the library, and not within the application. This allows the type @@ -28,9 +28,9 @@ Public Exported Function Definitions All exported functions must be declared as: ----- +```C GIT_EXTERN(result_type) git_modulename_functionname(arg_list); ----- +``` Semi-Private Exported Functions @@ -52,10 +52,10 @@ few arguments if multiple outputs are supplied). int status codes are 0 for GIT_OK and < 0 for an error. This permits common POSIX result testing: ----- +```C if (git_odb_open(&odb, path)) abort("odb open failed"); ----- +``` Functions returning a pointer may return NULL instead of an int if there is only one type of failure (GIT_ENOMEM). @@ -84,7 +84,7 @@ All public headers defining types, functions or macros must use the following form, where ${filename} is the name of the file, after replacing non-identifier characters with '_'. ----- +```C #ifndef INCLUDE_git_${filename}_h__ #define INCLUDE_git_${filename}_h__ @@ -104,4 +104,4 @@ after replacing non-identifier characters with '_'. /** @} */ GIT_END_DECL #endif ----- +``` From 24aec6db550062330a0a666df11ffc858cbffb33 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 21 Nov 2012 13:42:12 -0700 Subject: [PATCH 3/3] Rewrite conventions to be more complete --- CONVENTIONS.md | 198 +++++++++++++++++++++++++++++++------------------ 1 file changed, 127 insertions(+), 71 deletions(-) diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 06d3e875d..ea5e40ee6 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -1,107 +1,163 @@ -libgit2 conventions -=================== +# Libgit2 Conventions -Namespace Prefixes ------------------- - -All types and functions start with 'git_'. - -All #define macros start with 'GIT_'. +We like to keep the source consistent and readable. Herein are some guidelines +that should help with that. -Type Definitions ----------------- +## Naming Things -Most types should be opaque, e.g.: +All types and functions start with `git_`, and all #define macros start with `GIT_`. -```C - typedef struct git_odb git_odb; -``` +Functions with a single output parameter should name that parameter `out`. +Multiple outputs should be named `foo_out`, `bar_out`, etc. -with allocation functions returning an "instance" created within -the library, and not within the application. This allows the type -to grow (or shrink) in size without rebuilding client code. +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". -Public Exported Function Definitions ------------------------------------- +## Typedef + +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. + +## Exports All exported functions must be declared as: ```C - GIT_EXTERN(result_type) git_modulename_functionname(arg_list); +GIT_EXTERN(result_type) git_modulename_functionname(arg_list); ``` - -Semi-Private Exported Functions -------------------------------- +## Internals Functions whose modulename is followed by two underscores, -for example 'git_odb__read_packed', are semi-private functions. +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. +## Parameters -Calling Conventions -------------------- +Out parameters come first. -Functions should prefer to return a 'int' to indicate success or -failure and supply any output through the first argument (or first -few arguments if multiple outputs are supplied). +Whenever possible, pass argument pointers as `const`. Some structures (such +as `git_repository` and `git_index`) have internal structure that prevents +this. -int status codes are 0 for GIT_OK and < 0 for an error. -This permits common POSIX result testing: +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: ```C - if (git_odb_open(&odb, path)) - abort("odb open failed"); +int foo(git_repository *repo, git_foo_cb callback, void *payload); ``` -Functions returning a pointer may return NULL instead of an int -if there is only one type of failure (GIT_ENOMEM). -Functions returning a pointer may also return NULL if the common -case needed by the application is strictly success/failure and a -(possibly slower) function exists that the caller can use to get -more detailed information. Parsing common data structures from -on-disk formats is a good example of this pattern; in general a -"corrupt" entity can be treated as though it does not exist but -a more sophisticated "fsck" support function can report how the -entity is malformed. +## 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. -Documentation Fomatting ------------------------ +## Return codes -All comments should conform to Doxygen "javadoc" style conventions -for formatting the public API documentation. +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) +for the return codes already defined. + +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. -Public Header Format --------------------- +## Opaque Structs -All public headers defining types, functions or macros must use -the following form, where ${filename} is the name of the file, -after replacing non-identifier characters with '_'. +Most types should be opaque, e.g.: ```C - #ifndef INCLUDE_git_${filename}_h__ - #define INCLUDE_git_${filename}_h__ - - #include "git/common.h" - - /** - * @file git/${filename}.h - * @brief Git some description - * @defgroup git_${filename} some description routines - * @ingroup Git - * @{ - */ - GIT_BEGIN_DECL - - ... definitions ... - - /** @} */ - GIT_END_DECL - #endif +typedef struct git_odb git_odb; ``` + +...with allocation functions returning an "instance" created within +the library, and not within the application. This allows the type +to grow (or shrink) in size without rebuilding client code. + +To preserve ABI compatibility, include an `int version` field in all opaque +structures, and initialize to the latest version in the construction call. +Increment the "latest" version whenever the structure changes, and try to only +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: + +```C +git_foo_options opts = GIT_FOO_OPTIONS_INIT; +opts.baz = BAZ_OPTION_ONE; +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`. + +## 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: + +```C +GIT_EXTERN(int) git_foo_id( + git_oid **out, + int a, + int b); +``` + +Public headers are indented with spaces, three to a tab. Internal code is +indented with tabs; set your editor's tab width to 3 for best effect. + + +## 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. + + +## Public Header Template + +Use this template when creating a new public header. + +```C +#ifndef INCLUDE_git_${filename}_h__ +#define INCLUDE_git_${filename}_h__ + +#include "git/common.h" + +/** + * @file git/${filename}.h + * @brief Git some description + * @defgroup git_${filename} some description routines + * @ingroup Git + * @{ + */ +GIT_BEGIN_DECL + +/* ... definitions ... */ + +/** @} */ +GIT_END_DECL +#endif +``` + +