From db29ec7eee14cef6f256fbe7190f7b262fc0a472 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 15 Nov 2018 14:58:02 -0500 Subject: [PATCH] complete workflow document --- doc/github-development-workflow.md | 132 +++++++++++++++++++++++++---- 1 file changed, 117 insertions(+), 15 deletions(-) diff --git a/doc/github-development-workflow.md b/doc/github-development-workflow.md index b81dbb417e..31fd3f23f0 100644 --- a/doc/github-development-workflow.md +++ b/doc/github-development-workflow.md @@ -17,28 +17,28 @@ adapt accordingly. Last change 2018-11-15. * [Pull Request Discussions](#pull-request-discussions) * [Checklist for Pull Requests](#checklist-for-pull-requests) * [GitHub Issues](#github-issues) - * [Feature Request Issues](#feature-request-issues) - * [Bug Report Issues](#bug-report-issues) * [Milestones and Release Planning](#milestones-and-release-planning) ## GitHub Merge Management In the interest of consistency, ONLY ONE of the core LAMMPS developers -should doing the merging itself. This is currently @akohlmey (Axel -Kohlmeyer). If this assignment needs to be changed, it shall be done -right after a stable release. +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. ## Pull Requests ALL changes to the LAMMPS code and documentation, however trivial, MUST be submitted as a pull request to GitHub. All changes to the "master" branch must be made exclusively through merging pull requests. The -"unstable" and "stable" branches are only to be updated upon patch or -stable releases with fast-forward merges based on the associated -tags. Pull requests may also be submitted to (long-running) feature -branches created by LAMMPS developers inside the LAMMPS project, if -needed. Those are not subject to the merge and review restrictions -discussed in this document, though. +"unstable" and "stable" branches, respectively are only to be updated +upon patch or stable releases with fast-forward merges based on the +associated tags. Pull requests may also be submitted to (long-running) +feature branches created by LAMMPS developers inside the LAMMPS project, +if needed. Those are not subject to the merge and review restrictions +discussed in this document, though, but get manages as needed on a +case-by-case basis. ### Pull Request Assignments @@ -63,20 +63,122 @@ merged. ### Pull Request Reviews People can be assigned to review a pull request in two ways: + * They can be assigned manually to review a pull request by the submitter or a LAMMPS developer - * They can be automatically assigned, because -developer + * They can be automatically assigned, because a developers matches + a file pattern in the `.github/CODEOWNERS` file, which associates + developers with the code they contributed and maintain. + +Reviewers are requested to state their appraisal of the proposed changes +and either approve or request changes. People may unassign themselves +from review, if they feel not competent about the changes proposed. At +least one review from a LAMMPS developer with write access is required +before merging in addition to the automated compilation tests. The +feature, that reviews from code owners are "hard" reviews (i.e. they +must all be approved before merging is allowed), is currently disabled +and it is in the discretion of the merge maintainer to assess when +a sufficient degree of approval has been reached. Reviews may be +(automatically) dismissed, when the reviewed code has been changed, +and then approval is required a second time. ### Pull Request Discussions +All discussions about a pull request should be kept as much as possible +on the pull request discussion page on GitHub, so that other developers +can later review the entire discussion after the fact and understand the +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.txt`) + * 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_*.txt`, `doc/src/{fixes,computes,pairs,bonds,...}.txt` + and `doc/src/lammps.book` + * new 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 ` instead of `#include `. + 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 + * omplex.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 should follow the C++-98 standard. C++-11 is only accepted + in individual special purpose packages + * indentation is two spaces per level + * there should be no tabs and no trailing whitespace + * 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. + * 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. + ## GitHub Issues -### Feature Request 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. -### Bug Report Issues +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. ## 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_` 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 +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. +