Line data Source code
1 0 : \cond NEVER
2 : Distributed under the MIT License.
3 : See LICENSE.txt for details.
4 : \endcond
5 :
6 :
7 : # Contributing to SpECTRE {#contributing_to_spectre}
8 :
9 : \tableofcontents
10 :
11 : # Contributing to SpECTRE {#contributing_to_spectre_2}
12 :
13 : The following is a set of guidelines for contributing to SpECTRE,
14 : which is hosted in the [Simulating eXtreme Spacetimes
15 : Organization](https://github.com/sxs-collaboration) on GitHub.
16 :
17 : ## Code of Conduct
18 :
19 : This project and everyone participating in it is governed by the \ref
20 : code_of_conduct "SpECTRE Code of Conduct". By participating, you are
21 : expected to uphold this code. Please report possible violations of the
22 : code of conduct to conduct@spectre-code.org.
23 :
24 : ## What should I know before I get started? {#getting-started}
25 :
26 : SpECTRE is being developed in support of our collaborative Simulating
27 : eXtreme Spacetimes (SXS) research program into the multi-messenger
28 : astrophysics of neutron star mergers, core-collapse supernovae, and
29 : gamma-ray bursts. As such, almost all of the current contributors to
30 : SpECTRE are members of SXS institutions, and a large amount of
31 : discussion about SpECTRE is done in internal SXS meetings. If you are
32 : a member of SXS and wish to get involved, please contact one of the
33 : project leaders.
34 :
35 : In the future, we hope that SpECTRE can be applied to problems across
36 : discipline boundaries, and that it can be a true community code. At
37 : the present time, however, SpECTRE cannot yet solve realistic
38 : problems, and broad overview documentation is in an early, incomplete
39 : stage. Therefore, if you are not a member of SXS, but are interested
40 : in contributing to SpECTRE, we strongly encourage you to contact us at
41 : questions@spectre-code.org to discuss possible contributions.
42 :
43 : ## How Can I Contribute? {#how-can-i-contribute}
44 :
45 : ### Reporting Bugs {#reporting-bugs}
46 :
47 : This section guides you through submitting a bug report for
48 : SpECTRE. Following these guidelines helps maintainers and the
49 : community understand your report, reproduce the behavior, and find
50 : related reports.
51 :
52 : Before creating bug reports, please **perform a
53 : [search](https://github.com/sxs-collaboration/spectre/issues)** to see
54 : if the problem has already been reported. If it has **and the issue is
55 : still open**, please add a comment to the existing issue instead of
56 : opening a new one.
57 :
58 : > **Note:** If you find a **Closed** issue that seems like it is the
59 : > same thing that you're experiencing, please open a new issue and
60 : > include a link to the original issue in the body of your new one.
61 :
62 : #### How Do I Submit A (Good) Bug Report? {#submit-bug-report}
63 :
64 : Bugs are tracked as [GitHub
65 : issues](https://guides.github.com/features/issues/). When you are
66 : creating a bug report, please **include as many details as
67 : possible**. Please fill out the template completely. The provided
68 : information helps us resolve issues faster.
69 :
70 : Explain the problem and include additional details to help maintainers
71 : reproduce the problem:
72 :
73 : * **Use a clear and descriptive title** for the issue to identify the
74 : problem.
75 : * **Describe the exact steps which reproduce the problem** in as much
76 : detail as possible. For example, start by explaining how you started
77 : SpECTRE, e.g. the exact command you used in the terminal, or the
78 : contents of the batch job script you used.
79 : * **Describe the behavior you observed after following the steps** and
80 : point out what exactly is the problem with that behavior.
81 : * **Explain which behavior you expected to see instead and why.**
82 :
83 : Provide more context by answering these questions:
84 :
85 : * **Can you reproduce the problem in both debug and release mode?**
86 : (this is controlled by the CMake flag `CMAKE_BUILD_TYPE`)
87 : * **Did the problem start happening recently** (e.g. after updating to
88 : a new version/commit of SpECTRE) or was this always a problem?
89 : * If the problem started happening recently, **can you reproduce the
90 : problem in an older version/commit?** What's the most recent
91 : version/commit in which the problem doesn't happen?
92 : * **Can you reliably reproduce the issue?** If not, provide details
93 : about how often the problem happens and under which conditions it
94 : normally happens.
95 : * **Can you reproduce the problem on another machine?**
96 : * **Can you reproduce the problem in the docker container?** (see the
97 : \ref installation "Installation notes")
98 :
99 : Include details about your configuration and environment:
100 :
101 : * **Add as an attachment** (or add the contents of) the following:
102 : - The text output by SpECTRE (including any stack trace)
103 : - The input file(s)
104 : - $SPECTRE_BUILD_DIR/LibraryVersions.txt
105 : * **What is the name and version of the OS you're using**?
106 : * If possible (for SXS computers or HPC systems), a **path to a run
107 : directory** that is accessible by SpECTRE core developers.
108 :
109 : ### Suggesting Enhancements {#suggesting-enhancements}
110 :
111 : This section guides you through submitting an enhancement suggestion
112 : for SpECTRE, including completely new features and minor improvements
113 : to existing functionality. Following these guidelines helps
114 : maintainers and the community understand your suggestion and find
115 : related suggestions.
116 :
117 : Before creating enhancement suggestions, please **perform a
118 : [search](https://github.com/sxs-collaboration/spectre/issues)** to see
119 : if the enhancement has already been suggested. If it has, add a
120 : comment to the existing issue instead of opening a new one.
121 :
122 : #### How Do I Submit A (Good) Enhancement Suggestion? {#submit-enhancement}
123 :
124 : Enhancement suggestions are tracked as [GitHub
125 : issues](https://guides.github.com/features/issues/). When you are
126 : creating an enhancement suggestion, please **include as many details
127 : as possible** as you fill in the template.
128 :
129 : * **Use a clear and descriptive title** for the issue to identify the
130 : suggestion.
131 : * **Provide a step-by-step description of the suggested enhancement**
132 : in as many details as possible.
133 : * **Explain why this enhancement would be useful** to most SpECTRE users.
134 :
135 : ### Your First Code Contribution {#first-code-contribution}
136 :
137 : Unsure where to begin contributing to SpECTRE? You can start by
138 : looking through these `good first issue` and `help wanted` issues:
139 :
140 : * [Good first issues][good-first-issue] - issues which should only
141 : require a few lines of code, and a test or two.
142 : * [Help wanted issues][help-wanted] - issues which should be a bit
143 : more involved than `good first issue`s.
144 :
145 : #### Local development {#local-development}
146 :
147 : SpECTRE can be developed locally. For instructions on how to do this,
148 : see the following sections in the [SpECTRE
149 : documentation](https://spectre-code.org/):
150 :
151 : * [Installing SpECTRE](installation.html)
152 : * [Running Status Checks Locally](travis_guide.html#perform-checks-locally)
153 :
154 : ## Pull Requests {#pull-requests}
155 :
156 : Code contributions to SpECTRE follow a [pull request
157 : model](https://help.github.com/articles/about-pull-requests/)
158 :
159 : The process described here has several goals:
160 :
161 : - Maintain SpECTRE's code and documentation quality
162 : - Reach science goals in a timely manner
163 : - Fix problems that are important to users
164 : - Engage the community in working toward the best possible code
165 : - Enable a sustainable system for SpECTRE's maintainers to review
166 : contributions
167 :
168 : Please follow these steps to have your contribution considered by the
169 : maintainers:
170 :
171 : 1. Follow the \ref code_review_guide "code review guidelines", the
172 : \ref writing_unit_tests "guide to writing unit tests", and the \ref
173 : writing_good_dox "guide to writing documentation"
174 : 2. Follow all instructions in the pull request template. Reference
175 : related issues and pull requests.
176 : 3. After you submit your pull request, verify that all [status
177 : checks](https://help.github.com/articles/about-status-checks/) are
178 : passing
179 :
180 : > If a status check is failing, and you believe that the failure is
181 : > unrelated to your change, please leave a comment on the pull
182 : > request explaining why you believe the failure is unrelated. A
183 : > maintainer will re-run the status check for you. If we conclude
184 : > that the failure was a false positive, then we will open an issue
185 : > to track that problem with our status check suite.
186 :
187 : Only those status check failures that occur in the containerized build
188 : environment are your responsibility to fix. If you encounter an issue with a
189 : status check that runs in an environment that you do not have access to, e.g.
190 : on macOS or on a supercomputer, please notify
191 : `@sxs-collaboration/spectre-core-devs`. They will refer the issue to a person
192 : who has access to that environment. Unless requested by the reviewers, the PR
193 : will not be held up until the issue is resolved.
194 :
195 : While the prerequisites above must be satisfied prior to having your
196 : pull request reviewed, the reviewers may ask you to complete
197 : additional design work, tests, or other changes before your pull
198 : request can be ultimately accepted.
199 :
200 : ## How SpECTRE pull request reviews are conducted {#pull-request-reviews}
201 :
202 : > Note that these are guidelines and not rigid rules.
203 :
204 : Please make your pull requests as small as reasonably possible, as
205 : smaller pull requests are easier to review. In general, longer pull
206 : requests take longer to review, with the time scaling exponentially
207 : with the number of lines changed. Therefore if your pull request is
208 : too large, we may ask you to break it up into several smaller pull
209 : requests.
210 :
211 : > Below, days mean business days, so if the time period includes the
212 : > weekend, add two days, and for major holidays add a day.
213 : > Furthermore, most of us are academics, and we occasionally go to
214 : > conferences which may lead to delays in the review process. Also do
215 : > not expect much to happen between December 20th and January 3rd.
216 :
217 : Most pull requests submitted to SpECTRE will be reviewed in the
218 : following manner:
219 : - When creating the pull request, the author should add the
220 : appropriate labels. If the pull request is not a `new design` or
221 : `in progress` (discussed below), the author may either assign two
222 : reviewers (if GitHub suggested any) or add the `reviewers wanted`
223 : label.
224 : - Within two days, assigned reviewers should either confirm that they
225 : are able to review, or remove themselves while adding the `reviewers
226 : wanted` label.
227 : - Anyone is welcome to self-assign themselves as a reviewer; when
228 : there are at least two reviewers, the `reviewers wanted` label
229 : should be removed.
230 : - If there are not two reviewers after two days, a [SpECTRE core
231 : developer](#core-developers) will assign reviewers, removing the
232 : `reviewers wanted` label. If for some reason, no reviewers have
233 : been assigned after three days, the author of the pull request
234 : should feel free to [ping the core
235 : developers](https://github.blog/2011-03-23-mention-somebody-they-re-notified/)
236 : (e.g `@sxs-collaboration/spectre-core-devs please assign reviewers`)
237 : - Assigned reviewers should submit their review in as timely a manner
238 : as possible.
239 : - Anyone can request changes within either the first two days of the
240 : pull request, or within a day after the initial reviews of the
241 : assigned reviewers. After this period, only the assigned reviewers
242 : can request changes, unless someone believes the code is wrong.
243 : Non-reviewers are allowed to make comments, which the pull request
244 : author is encouraged to address. Alternatively the author can
245 : create an issue with the suggested changes, assigned to themselves,
246 : which would then be addressed in a subsequent pull request by the
247 : author.
248 : - If any requested change is unclear to the author, they should ping
249 : the reviewer and ask for clarification. Authors and reviewers are
250 : encouraged to talk to one another (in person, via Google hangout, or
251 : some other verbal method if possible) to resolve any issues.
252 : - Reviewers are encouraged to [ping others
253 : \@GITHUB_USERNAME](https://github.blog/2011-03-23-mention-somebody-they-re-notified/)
254 : to get opinions on code they are unsure about.
255 : - It is permissible to have a group code review led by one of the
256 : reviewers. The reviewer should comment on who was present at the
257 : group review.
258 : - If necessary, pull requests can also be discussed at one of the
259 : weekly SpECTRE meetings.
260 : - If changes are requested, the author should fix all of them in one
261 : or more fixup commits (where the first line of the commit message
262 : should begin with `fixup`), and then ping the reviewers that the
263 : pull request is updated. In addition the `updated` label can be
264 : added.
265 : - Once a pull request is updated, the reviewers should either request
266 : further changes (removing the `updated` label) or tell the pull
267 : request author to squash their commits.
268 : - Once all reviewers give the okay to squash, the author should
269 : [rebase on develop and then squash their
270 : commits](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History)
271 : into one or more self-contained commits (such that the code will
272 : compile and pass all tests after each commit). The squashed commits
273 : will need to be force pushed.
274 : - Once the commits are squashed, the author should ping the reviewers.
275 : - Once all reviewers have approved the pull request, someone should
276 : ping the `@sxs-collaboration/spectre-core-devs`. If one of the
277 : original reviewers is a core developer, this is not necessary and
278 : the core developer can merge the pull request.
279 : - One of the core developers will perform a final cursory review,
280 : requesting changes only for major problems, and commenting on other
281 : possible changes.
282 : - The pull request author should address all final requested changes
283 : and may either also fix final suggested changes, or create an issue
284 : with the suggested changes, which will be addressed in a subsequent
285 : pull request by the author.
286 : - The SpECTRE core developer will merge the pull request once all comments have
287 : been addressed, all reviewers have approved the PR, the code passes CI and all
288 : pull requests the pull request depends on have been merged. When approvals are
289 : dismissed by minor changes, such as rebasing, squashing fixup commits or
290 : adding a missing include, the core developer may merge the PR without waiting
291 : for all reviewers to re-approve the PR.
292 :
293 : Critical bug fixes (i.e. the code is broken) can be merged after two
294 : expedited reviews by SpECTRE core developers. If necessary, an issue
295 : can be created if further changes are desired.
296 :
297 : Pull requests that are designated `new design` are expected to have a
298 : longer review period, including discussions during weekly SpECTRE
299 : meetings. SpECTRE core developers will provide reasonable review
300 : deadlines once the new design is finalized.
301 :
302 : If you would like feedback on a pull request prior to it being ready
303 : for formal review, please label it with `in progress` and ping
304 : whomever you wish to get feedback from. As long as the `in progress`
305 : label remains, no one should review the pull request.
306 :
307 :
308 : ### Git Commit Message Guidelines {#git-commit-messages}
309 :
310 : * Use the present tense ("Add feature" not "Added feature")
311 : * Use the imperative mood ("Move cursor to..." not "Moves cursor to...")
312 : * Limit the first line to 72 characters or less
313 : * If needed, a blank second line followed by a more complete description
314 :
315 : ### SpECTRE core developers {#core-developers}
316 :
317 : SpECTRE core developers are people who are very familiar with the
318 : entire code, comfortable with modern C++, and willing to take the
319 : responsibility of overseeing the code as a whole.
320 : [Current SpECTRE core
321 : developers](https://github.com/orgs/sxs-collaboration/teams/spectre-core-devs)
322 : can be pinged on GitHub at `@sxs-collaboration/spectre-core-devs`. It
323 : is expected that as more contributors become familiar with SpECTRE,
324 : additional people will be added to the list of core developers.
325 :
326 :
327 : [good-first-issue]:https://github.com/sxs-collaboration/spectre/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22+sort%3Acomments-desc
328 : [help-wanted]:https://github.com/sxs-collaboration/spectre/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22+sort%3Acomments-desc
329 :
|