diff options
author | thegeorg <thegeorg@yandex-team.ru> | 2022-02-10 16:45:08 +0300 |
---|---|---|
committer | Daniil Cherednik <dcherednik@yandex-team.ru> | 2022-02-10 16:45:08 +0300 |
commit | 4e839db24a3bbc9f1c610c43d6faaaa99824dcca (patch) | |
tree | 506dac10f5df94fab310584ee51b24fc5a081c22 /contrib/libs/zstd/CONTRIBUTING.md | |
parent | 2d37894b1b037cf24231090eda8589bbb44fb6fc (diff) | |
download | ydb-4e839db24a3bbc9f1c610c43d6faaaa99824dcca.tar.gz |
Restoring authorship annotation for <thegeorg@yandex-team.ru>. Commit 1 of 2.
Diffstat (limited to 'contrib/libs/zstd/CONTRIBUTING.md')
-rw-r--r-- | contrib/libs/zstd/CONTRIBUTING.md | 226 |
1 files changed, 113 insertions, 113 deletions
diff --git a/contrib/libs/zstd/CONTRIBUTING.md b/contrib/libs/zstd/CONTRIBUTING.md index e7e545129e..de159064f8 100644 --- a/contrib/libs/zstd/CONTRIBUTING.md +++ b/contrib/libs/zstd/CONTRIBUTING.md @@ -5,7 +5,7 @@ possible. ## Our Development Process New versions are being developed in the "dev" branch, or in their own feature branch. -When they are deemed ready for a release, they are merged into "release". +When they are deemed ready for a release, they are merged into "release". As a consequences, all contributions must stage first through "dev" or their own feature branch. @@ -126,20 +126,20 @@ just `contrib/largeNbDicts` and nothing else, you can run: scan-build make -C contrib/largeNbDicts largeNbDicts ``` -### Pitfalls of static analysis -`scan-build` is part of our regular CI suite. Other static analyzers are not. - -It can be useful to look at additional static analyzers once in a while (and we do), but it's not a good idea to multiply the nb of analyzers run continuously at each commit and PR. The reasons are : - -- Static analyzers are full of false positive. The signal to noise ratio is actually pretty low. -- A good CI policy is "zero-warning tolerance". That means that all issues must be solved, including false positives. This quickly becomes a tedious workload. -- Multiple static analyzers will feature multiple kind of false positives, sometimes applying to the same code but in different ways leading to : - + torteous code, trying to please multiple constraints, hurting readability and therefore maintenance. Sometimes, such complexity introduce other more subtle bugs, that are just out of scope of the analyzers. - + sometimes, these constraints are mutually exclusive : if one try to solve one, the other static analyzer will complain, they can't be both happy at the same time. -- As if that was not enough, the list of false positives change with each version. It's hard enough to follow one static analyzer, but multiple ones with their own update agenda, this quickly becomes a massive velocity reducer. - -This is different from running a static analyzer once in a while, looking at the output, and __cherry picking__ a few warnings that seem helpful, either because they detected a genuine risk of bug, or because it helps expressing the code in a way which is more readable or more difficult to misuse. These kind of reports can be useful, and are accepted. - +### Pitfalls of static analysis +`scan-build` is part of our regular CI suite. Other static analyzers are not. + +It can be useful to look at additional static analyzers once in a while (and we do), but it's not a good idea to multiply the nb of analyzers run continuously at each commit and PR. The reasons are : + +- Static analyzers are full of false positive. The signal to noise ratio is actually pretty low. +- A good CI policy is "zero-warning tolerance". That means that all issues must be solved, including false positives. This quickly becomes a tedious workload. +- Multiple static analyzers will feature multiple kind of false positives, sometimes applying to the same code but in different ways leading to : + + torteous code, trying to please multiple constraints, hurting readability and therefore maintenance. Sometimes, such complexity introduce other more subtle bugs, that are just out of scope of the analyzers. + + sometimes, these constraints are mutually exclusive : if one try to solve one, the other static analyzer will complain, they can't be both happy at the same time. +- As if that was not enough, the list of false positives change with each version. It's hard enough to follow one static analyzer, but multiple ones with their own update agenda, this quickly becomes a massive velocity reducer. + +This is different from running a static analyzer once in a while, looking at the output, and __cherry picking__ a few warnings that seem helpful, either because they detected a genuine risk of bug, or because it helps expressing the code in a way which is more readable or more difficult to misuse. These kind of reports can be useful, and are accepted. + ## Continuous Integration CI tests run every time a pull request (PR) is created or updated. The exact tests that get run will depend on the destination branch you specify. Some tests take @@ -384,106 +384,106 @@ disclosure of security bugs. In those cases, please go through the process outlined on that page and do not file a public issue. ## Coding Style -It's a pretty long topic, which is difficult to summarize in a single paragraph. -As a rule of thumbs, try to imitate the coding style of -similar lines of codes around your contribution. -The following is a non-exhaustive list of rules employed in zstd code base: - -### C90 -This code base is following strict C90 standard, -with 2 extensions : 64-bit `long long` types, and variadic macros. -This rule is applied strictly to code within `lib/` and `programs/`. -Sub-project in `contrib/` are allowed to use other conventions. - -### C++ direct compatibility : symbol mangling -All public symbol declarations must be wrapped in `extern “C” { … }`, -so that this project can be compiled as C++98 code, -and linked into C++ applications. - -### Minimal Frugal -This design requirement is fundamental to preserve the portability of the code base. -#### Dependencies -- Reduce dependencies to the minimum possible level. - Any dependency should be considered “bad” by default, - and only tolerated because it provides a service in a better way than can be achieved locally. - The only external dependencies this repository tolerates are - standard C libraries, and in rare cases, system level headers. -- Within `lib/`, this policy is even more drastic. - The only external dependencies allowed are `<assert.h>`, `<stdlib.h>`, `<string.h>`, - and even then, not directly. - In particular, no function shall ever allocate on heap directly, - and must use instead `ZSTD_malloc()` and equivalent. - Other accepted non-symbol headers are `<stddef.h>` and `<limits.h>`. -- Within the project, there is a strict hierarchy of dependencies that must be respected. - `programs/` is allowed to depend on `lib/`, but only its public API. - Within `lib/`, `lib/common` doesn't depend on any other directory. - `lib/compress` and `lib/decompress` shall not depend on each other. - `lib/dictBuilder` can depend on `lib/common` and `lib/compress`, but not `lib/decompress`. -#### Resources -- Functions in `lib/` must use very little stack space, - several dozens of bytes max. - Everything larger must use the heap allocator, - or require a scratch buffer to be emplaced manually. - -### Naming -* All public symbols are prefixed with `ZSTD_` - + private symbols, with a scope limited to their own unit, are free of this restriction. - However, since `libzstd` source code can be amalgamated, - each symbol name must attempt to be (and remain) unique. - Avoid too generic names that could become ground for future collisions. - This generally implies usage of some form of prefix. -* For symbols (functions and variables), naming convention is `PREFIX_camelCase`. - + In some advanced cases, one can also find : - - `PREFIX_prefix2_camelCase` - - `PREFIX_camelCase_extendedQualifier` -* Multi-words names generally consist of an action followed by object: - - for example : `ZSTD_createCCtx()` -* Prefer positive actions - - `goBackward` rather than `notGoForward` -* Type names (`struct`, etc.) follow similar convention, - except that they are allowed and even invited to start by an Uppercase letter. - Example : `ZSTD_CCtx`, `ZSTD_CDict` -* Macro names are all Capital letters. - The same composition rules (`PREFIX_NAME_QUALIFIER`) apply. -* File names are all lowercase letters. - The convention is `snake_case`. - File names **must** be unique across the entire code base, - even when they stand in clearly separated directories. - -### Qualifiers -* This code base is `const` friendly, if not `const` fanatical. - Any variable that can be `const` (aka. read-only) **must** be `const`. - Any pointer which content will not be modified must be `const`. - This property is then controlled at compiler level. - `const` variables are an important signal to readers that this variable isn’t modified. - Conversely, non-const variables are a signal to readers to watch out for modifications later on in the function. -* If a function must be inlined, mention it explicitly, - using project's own portable macros, such as `FORCE_INLINE_ATTR`, - defined in `lib/common/compiler.h`. - -### Debugging -* **Assertions** are welcome, and should be used very liberally, - to control any condition the code expects for its correct execution. - These assertion checks will be run in debug builds, and disabled in production. -* For traces, this project provides its own debug macros, - in particular `DEBUGLOG(level, ...)`, defined in `lib/common/debug.h`. - -### Code documentation -* Avoid code documentation that merely repeats what the code is already stating. - Whenever applicable, prefer employing the code as the primary way to convey explanations. - Example 1 : `int nbTokens = n;` instead of `int i = n; /* i is a nb of tokens *./`. - Example 2 : `assert(size > 0);` instead of `/* here, size should be positive */`. -* At declaration level, the documentation explains how to use the function or variable - and when applicable why it's needed, of the scenarios where it can be useful. -* At implementation level, the documentation explains the general outline of the algorithm employed, - and when applicable why this specific choice was preferred. - -### General layout +It's a pretty long topic, which is difficult to summarize in a single paragraph. +As a rule of thumbs, try to imitate the coding style of +similar lines of codes around your contribution. +The following is a non-exhaustive list of rules employed in zstd code base: + +### C90 +This code base is following strict C90 standard, +with 2 extensions : 64-bit `long long` types, and variadic macros. +This rule is applied strictly to code within `lib/` and `programs/`. +Sub-project in `contrib/` are allowed to use other conventions. + +### C++ direct compatibility : symbol mangling +All public symbol declarations must be wrapped in `extern “C” { … }`, +so that this project can be compiled as C++98 code, +and linked into C++ applications. + +### Minimal Frugal +This design requirement is fundamental to preserve the portability of the code base. +#### Dependencies +- Reduce dependencies to the minimum possible level. + Any dependency should be considered “bad” by default, + and only tolerated because it provides a service in a better way than can be achieved locally. + The only external dependencies this repository tolerates are + standard C libraries, and in rare cases, system level headers. +- Within `lib/`, this policy is even more drastic. + The only external dependencies allowed are `<assert.h>`, `<stdlib.h>`, `<string.h>`, + and even then, not directly. + In particular, no function shall ever allocate on heap directly, + and must use instead `ZSTD_malloc()` and equivalent. + Other accepted non-symbol headers are `<stddef.h>` and `<limits.h>`. +- Within the project, there is a strict hierarchy of dependencies that must be respected. + `programs/` is allowed to depend on `lib/`, but only its public API. + Within `lib/`, `lib/common` doesn't depend on any other directory. + `lib/compress` and `lib/decompress` shall not depend on each other. + `lib/dictBuilder` can depend on `lib/common` and `lib/compress`, but not `lib/decompress`. +#### Resources +- Functions in `lib/` must use very little stack space, + several dozens of bytes max. + Everything larger must use the heap allocator, + or require a scratch buffer to be emplaced manually. + +### Naming +* All public symbols are prefixed with `ZSTD_` + + private symbols, with a scope limited to their own unit, are free of this restriction. + However, since `libzstd` source code can be amalgamated, + each symbol name must attempt to be (and remain) unique. + Avoid too generic names that could become ground for future collisions. + This generally implies usage of some form of prefix. +* For symbols (functions and variables), naming convention is `PREFIX_camelCase`. + + In some advanced cases, one can also find : + - `PREFIX_prefix2_camelCase` + - `PREFIX_camelCase_extendedQualifier` +* Multi-words names generally consist of an action followed by object: + - for example : `ZSTD_createCCtx()` +* Prefer positive actions + - `goBackward` rather than `notGoForward` +* Type names (`struct`, etc.) follow similar convention, + except that they are allowed and even invited to start by an Uppercase letter. + Example : `ZSTD_CCtx`, `ZSTD_CDict` +* Macro names are all Capital letters. + The same composition rules (`PREFIX_NAME_QUALIFIER`) apply. +* File names are all lowercase letters. + The convention is `snake_case`. + File names **must** be unique across the entire code base, + even when they stand in clearly separated directories. + +### Qualifiers +* This code base is `const` friendly, if not `const` fanatical. + Any variable that can be `const` (aka. read-only) **must** be `const`. + Any pointer which content will not be modified must be `const`. + This property is then controlled at compiler level. + `const` variables are an important signal to readers that this variable isn’t modified. + Conversely, non-const variables are a signal to readers to watch out for modifications later on in the function. +* If a function must be inlined, mention it explicitly, + using project's own portable macros, such as `FORCE_INLINE_ATTR`, + defined in `lib/common/compiler.h`. + +### Debugging +* **Assertions** are welcome, and should be used very liberally, + to control any condition the code expects for its correct execution. + These assertion checks will be run in debug builds, and disabled in production. +* For traces, this project provides its own debug macros, + in particular `DEBUGLOG(level, ...)`, defined in `lib/common/debug.h`. + +### Code documentation +* Avoid code documentation that merely repeats what the code is already stating. + Whenever applicable, prefer employing the code as the primary way to convey explanations. + Example 1 : `int nbTokens = n;` instead of `int i = n; /* i is a nb of tokens *./`. + Example 2 : `assert(size > 0);` instead of `/* here, size should be positive */`. +* At declaration level, the documentation explains how to use the function or variable + and when applicable why it's needed, of the scenarios where it can be useful. +* At implementation level, the documentation explains the general outline of the algorithm employed, + and when applicable why this specific choice was preferred. + +### General layout * 4 spaces for indentation rather than tabs -* Code documentation shall directly precede function declaration or implementation -* Function implementations and its code documentation should be preceded and followed by an empty line - +* Code documentation shall directly precede function declaration or implementation +* Function implementations and its code documentation should be preceded and followed by an empty line + ## License By contributing to Zstandard, you agree that your contributions will be licensed under both the [LICENSE](LICENSE) file and the [COPYING](COPYING) file in the root directory of this source tree. |