update style guide and requirements/suggestions for contributions

This commit is contained in:
Axel Kohlmeyer
2021-09-07 19:09:35 -04:00
parent 19e6a9e0d8
commit ad39aa85ab
10 changed files with 608 additions and 506 deletions

View File

@ -6,7 +6,7 @@ choices the LAMMPS developers have agreed on. Git and GitHub provide the
tools, but do not set policies, so it is up to the developers to come to
an agreement as to how to define and interpret policies. This document
is likely to change as our experiences and needs change and we try to
adapt accordingly. Last change 2018-12-19.
adapt accordingly. Last change 2021-09-02.
## Table of Contents
@ -23,10 +23,10 @@ adapt accordingly. Last change 2018-12-19.
In the interest of consistency, ONLY ONE of the core LAMMPS developers
should doing the merging itself. This is currently
[@akohlmey](https://github.com/akohlmey) (Axel Kohlmeyer).
If this assignment needs to be changed, it shall be done right after a
stable release. If the currently assigned developer cannot merge outstanding pull
requests in a timely manner, or in other extenuating circumstances,
[@akohlmey](https://github.com/akohlmey) (Axel Kohlmeyer). If this
assignment needs to be changed, it shall be done right after a stable
release. If the currently assigned developer cannot merge outstanding
pull requests in a timely manner, or in other extenuating circumstances,
other core LAMMPS developers with merge rights can merge pull requests,
when necessary.
@ -55,13 +55,14 @@ the required changes or ask the submitter of the pull request to implement
them. Even though, all LAMMPS developers may have write access to pull
requests (if enabled by the submitter, which is the default), only the
submitter or the assignee of a pull request may do so. During this
period the `work_in_progress` label shall be applied to the pull
period the `work_in_progress` label may be applied to the pull
request. The assignee gets to decide what happens to the pull request
next, e.g. whether it should be assigned to a different developer for
additional checks and changes, or is recommended to be merged. Removing
the `work_in_progress` label and assigning the pull request to the
developer tasked with merging signals that a pull request is ready to be
merged.
merged. In addition, a `ready_for_merge` label may also be assigned
to signal urgency to merge this pull request quickly.
### Pull Request Reviews
@ -97,108 +98,50 @@ rationale behind choices made. Exceptions to this policy are technical
discussions, that are centered on tools or policies themselves
(git, GitHub, c++) rather than on the content of the pull request.
### Checklist for Pull Requests
Here are some items to check:
* source and text files should not have CR/LF line endings (use dos2unix to remove)
* every new command or style should have documentation. The names of
source files (c++ and manual) should follow the name of the style.
(example: `src/fix_nve.cpp`, `src/fix_nve.h` for `fix nve` command,
implementing the class `FixNVE`, documented in `doc/src/fix_nve.rst`)
* all new style names should be lower case, the must be no dashes,
blanks, or underscores separating words, only forward slashes.
* new style docs should be added to the "overview" files in
`doc/src/Commands_*.rst`, `doc/src/{fixes,computes,pairs,bonds,...}.rst`
* check whether manual cleanly translates with `make html` and `make pdf`
* if documentation is (still) provided as a .txt file, convert to .rst
and remove the .txt file. For files in doc/txt the conversion is automatic.
* remove all .txt files in `doc/txt` that are out of sync with their .rst counterparts in `doc/src`
* check spelling of manual with `make spelling` in doc folder
* check style tables and command lists with `make style_check`
* new source files in packages should be added to `src/.gitignore`
* removed or renamed files in packages should be added to `src/Purge.list`
* C++ source files should use C++ style include files for accessing
C-library APIs, e.g. `#include <cstdlib>` instead of `#include <stdlib.h>`.
And they should use angular brackets instead of double quotes. Full list:
* assert.h -> cassert
* ctype.h -> cctype
* errno.h -> cerrno
* float.h -> cfloat
* limits.h -> climits
* math.h -> cmath
* complex.h -> complex
* setjmp.h -> csetjmp
* signal.h -> csignal
* stddef.h -> cstddef
* stdint.h -> cstdint
* stdio.h -> cstdio
* stdlib.h -> cstdlib
* string.h -> cstring
* time.h -> ctime
* Do NOT replace (as they are C++-11): `inttypes.h` and `stdint.h`.
* Code must follow the C++-11 standard. C++98-only is no longer accepted
* Code should use `nullptr` instead of `NULL` where applicable.
in individual special purpose packages
* indentation is 2 spaces per level
* there should be NO tabs and no trailing whitespace (review the "checkstyle" test on pull requests)
* header files, especially of new styles, should not include any
other headers, except the header with the base class or cstdio.
Forward declarations should be used instead when possible.
* iostreams should be avoided. LAMMPS uses stdio from the C-library.
* use of STL in headers and class definitions should be avoided.
exception is <string>, but it won't need to be explicitly included
since pointers.h already includes it. so std::string can be used directly.
* there MUST NOT be any "using namespace XXX;" statements in headers.
* static class members should be avoided at all cost.
* anything storing atom IDs should be using `tagint` and not `int`.
This can be flagged by the compiler only for pointers and only when
compiling LAMMPS with `-DLAMMPS_BIGBIG`.
* when including both `lmptype.h` (and using defines or macros from it)
and `mpi.h`, `lmptype.h` must be included first.
* see https://github.com/lammps/lammps/blob/master/doc/include-file-conventions.md
for general include file conventions and best practices
* when pair styles are added, check if settings for flags like
`single_enable`, `writedata`, `reinitflag`, `manybody_flag`
and others are correctly set and supported.
## GitHub Issues
The GitHub issue tracker is the location where the LAMMPS developers
and other contributors or LAMMPS users can report issues or bugs with
the LAMMPS code or request new features to be added. Feature requests
are usually indicated by a `[Feature Request]` marker in the subject.
Issues are assigned to a person, if this person is working on this
feature or working to resolve an issue. Issues that have nobody working
on them at the moment, have the label `volunteer needed` attached.
the LAMMPS code or request new features to be added. Bug reports have
a `[Bug]` marker in the subject line; suggestions for changes or
adding new functionality are indicated by a `[Feature Request]`
marker in the subject. This is automatically done when using the
corresponding template for submitting an issue. Issues may be assigned
to one or more developers, if they are working on this feature or
working to resolve an issue. Issues that have nobody working
on them at the moment or in the near future, have the label
`volunteer needed` attached.
When an issue, say `#125` is resolved by a specific pull request,
the comment for the pull request shall contain the text `closes #125`
or `fixes #125`, so that the issue is automatically deleted when
the pull request is merged.
the pull request is merged. The template for pull requests includes
a header where connections between pull requests and issues can be listed
and thus were this comment should be placed.
## Milestones and Release Planning
LAMMPS uses a continuous release development model with incremental
changes, i.e. significant effort is made - including automated pre-merge
testing - that the code in the branch "master" does not get broken.
More extensive testing (including regression testing) is performed after
code is merged to the "master" branch. There are patch releases of
LAMMPS every 1-3 weeks at a point, when the LAMMPS developers feel, that
a sufficient amount of changes have happened, and the post-merge testing
has been successful. These patch releases are marked with a
`patch_<version date>` tag and the "unstable" branch follows only these
versions (and thus is always supposed to be of production quality,
unlike "master", which may be temporary broken, in the case of larger
change sets or unexpected incompatibilities or side effects.
testing - that the code in the branch "master" does not get easily
broken. These tests are run after every update to a pull request. More
extensive and time consuming tests (including regression testing) are
performed after code is merged to the "master" branch. There are patch
releases of LAMMPS every 3-5 weeks at a point, when the LAMMPS
developers feel, that a sufficient amount of changes have happened, and
the post-merge testing has been successful. These patch releases are
marked with a `patch_<version date>` tag and the "unstable" branch
follows only these versions (and thus is always supposed to be of
production quality, unlike "master", which may be temporary broken, in
the case of larger change sets or unexpected incompatibilities or side
effects.
About 3-4 times each year, there are going to be "stable" releases
of LAMMPS. These have seen additional, manual testing and review of
About 1-2 times each year, there are going to be "stable" releases of
LAMMPS. These have seen additional, manual testing and review of
results from testing with instrumented code and static code analysis.
Also, in the last 2-3 patch releases before a stable release are
"release candidate" versions which only contain bugfixes and
documentation updates. For release planning and the information of
code contributors, issues and pull requests being actively worked on
are assigned a "milestone", which corresponds to the next stable
release or the stable release after that, with a tentative release
date.
Also, the last 1-3 patch releases before a stable release are "release
candidate" versions which only contain bugfixes and documentation
updates. For release planning and the information of code contributors,
issues and pull requests being actively worked on are assigned a
"milestone", which corresponds to the next stable release or the stable
release after that, with a tentative release date.