7.46 KB
Newer Older
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41
Contributing to Nektar++

## Issues and bug reports
Think you've found a bug or issue with Nektar++? We're very keen to hear about
- In the first instance, you should raise an issue on the
  **[issue tracker](** -- be
  sure to do a quick search and see if anyone has reported the same thing first.
- Alternatively you can
  **[join the mailing list](**
  for more advice.

It's *really helpful* if you can include a small session file that reproduces
the error, and can give a good description of the problem you're having.

## How to contribute
If you've got a patch or feature, please consider contributing it back to the
project. It's a pretty simple process:

1. Fork the Nektar++ repository in `nektar/nektar` into your username's space.
2. Create a branch with the naming convention:
   - `feature/myawesomebranch`: a new feature that wasn't in Nektar++ already.
   - `fix/mygreatfix`: fixes an issue that isn't tracked in the issue tracker.
   - `ticket/123-myfantasticpatch`: fixes an issue that is tracked in the issue
     tracker (please include the issue number somewhere!)
   - `tidy/mybrillianttidying`: cosmetic fixes to bring existing files up to the
     Nektar++ code guidelines.
3. Make sure you've gone through the checklist below.
4. Submit a merge request to merge into `master`. If you just want to see the
   diff and are not quite ready to merge, use the `[WIP]` tag in the title to
   prevent your code from being accidentally merged.
5. Put a comment in the MR saying that it's ready to be merged.
6. Respond to any comments in the code review.

## Submission checklist
- Did you add regression tests (for fixes) or unit tests and/or normal tests for
  new features?
- Have you run your branch through buildbot and do all the tests pass?
- Is there documentation in the user guide and/or developer guide?
- Are there any massive files you might have added in the commit history? We try
42 43 44 45 46 47 48 49
  to keep test files as small as possible. If so you'll need to rebase or
  filter-branch to remove those from the commit history.
- Is the code formatted correctly?
  - **Note:** unfortunately, Nektar++ has pretty inconsistent code formatting at
    the moment. To help in reviewing your submission, new files should be
    formatted according to the guidelines (or use `clang-format` as described
    below) -- otherwise, try to keep formatting consistent with the file you're
    working on.
50 51 52 53 54

## Testing and Buildbot
Your new features or fixes should include tests that cover the code you've
added. There are numerous examples within the various `Tests` directory lying
within the source trees, and there is an example of writing `.tst` files for our
55 56 57 58
`Tester` executable in the `tests/Examples` directory. Once you've written your
tests, add them to the `CMakeLists.txt` file for the relevant solver, or to the
appropriate demos directory for library features in whatever directory you are
working in.
59 60 61 62 63 64 65 66

You should also test your branch on the
[Nektar++ buildbot](, which will compile and test
the code against a number of Linux, Mac and Windows operating systems, both 32-
and 64-bit. If your tests don't pass, we can't merge the code into master.

## Documentation
Nektar++ has a fairly comprehensive user guide and a developer guide that is
67 68 69 70 71 72 73 74
presently very incomplete. The following are rough guidelines for what you
should provide:

- If you are writing user-exposed features, you should add some documentation to
  the user guide on how to use them.
- Any functions/classes should include Doxygen documentation.
- Generally, code should be well-commented using regular C++ comments to explain
  its function to help in reviewing it.
75 76 77 78 79 80 81 82

## Code review and merging
All merge requests will be reviewed by one of the senior developers. We try to
stick to the following process:
- Senior developer will be assigned, MR will be assigned a milestone to target a
  - If the branch is deemed to be minor and passes the checklist above, senior
    developer will handle the request by themselves.
83 84 85
  - Otherwise, senior developer will ask one or more other developers to review
    the code.
- Submission checklist will be checked by the reviewers.
86 87
- Where appropriate, reviewers will comment on regions of code that need further
  development and/or improvement.
88 89 90
- In addition to any coding comments/suggestions, reviewers are asked to check
  the branch passes the regression tests and appropriate documentation has been
91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109
- Once feedback received from the branch author (if necessary) and reviewers are
  happy, the branch will be merged.

## Formatting guidelines
Nektar++ uses C++, a language notorious for being easy to make obtuse and
difficult to follow code. To hopefully alleviate this problem, there are a
number of fairly simple formatting guidelines you should follow. We are
reasonably relaxed about code formatting, but if you can follow the guidelines
below this would be fantastic.

### Basic rules
- All code should be wrapped to 80 characters.
- Indentation should be 4 spaces with **no tabs**. Namespaces should not be
  indented to give more room in the 80 character width.
- Please comment your code with Doxygen and inline comments wherever possible --
  but don't use trailing inline comments to save the 80 character limit!
- All code blocks (even one-line blocks) should use braces, and braces should be
  on new lines; for instance
110 111
  if (someCondition)
112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137
- **Don't use preprocessor directives and macros unless there is no viable
  alternative.** The exception to this is header guards inside your `.h` files.
- Use one `.cpp` and `.h` file per C++ class, and try to keep `inline` header
  code to a minimum (unless performance is a major factor).
- Put spaces around binary operators and constants.
- Put spaces after `if`, `while`, etc., but not after function names (see the
  example above).

### Variables and naming
- Please use sensible names!
- Use camelCase to define your variable and function names,
  e.g. `myAwesomeVariable`. Any `typedef`s, function, `class` and `struct` names
  should begin with capital letters, variable names should be lower case.
- Inside classes, member variables should be prefixed with `m_`,
  e.g. `m_myAwesomeVariable`.
  - Global constants used throughout the library should be prefixed with `k`
    (e.g. `kGeometricTolerance`), and enumerations should be prefixed with `e`
    (e.g. `eGeometry`).
- Use all uppercase letters with underscores between words for pre-processor
  definitions and macros.

### Using `clang-format`
Code formatting is reasonably boring, so Nektar++ comes with a `.clang-format`
138 139 140 141 142 143
file to allow for automatic code formatting. As noted above, you can use this
for new files, or cosmetic `tidy/*` branches, but try to stick to existing
formatting elsewhere.

Installing it is easy on most package managers. Nektar++ relies on options that
are used in version 3.7 or later.
144 145 146 147 148 149 150 151 152 153 154 155

There are a number of instructions on how to use `clang-format` inside a number
of text editors on the
[CLang website]( However at a
minimum, you should consider downloading the
script into one of your `$PATH` locations. You can then run the command

    git clang-format

before you do a `git commit`, and `clang-format` will automatically format your
diff according to the guidelines.