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/BuildInfo.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
153 : Locally](github_actions_guide.html#perform-checks-locally)
154 :
155 : ## Pull Requests {#pull-requests}
156 :
157 : Code contributions to SpECTRE follow a [pull request
158 : model](https://help.github.com/articles/about-pull-requests/)
159 :
160 : The process described here has several goals:
161 :
162 : - Maintain SpECTRE's code and documentation quality
163 : - Reach science goals in a timely manner
164 : - Fix problems that are important to users
165 : - Engage the community in working toward the best possible code
166 : - Enable a sustainable system for SpECTRE's maintainers to review
167 : contributions
168 :
169 : Please follow these steps to have your contribution considered by the
170 : maintainers:
171 :
172 : 1. Follow the \ref code_review_guide "code review guidelines", the
173 : \ref writing_unit_tests "guide to writing unit tests", and the \ref
174 : writing_good_dox "guide to writing documentation"
175 : 2. Follow all instructions in the pull request template. Reference
176 : related issues and pull requests.
177 : 3. After you submit your pull request, verify that all [status
178 : checks](https://help.github.com/articles/about-status-checks/) are
179 : passing
180 :
181 : > If a status check is failing, and you believe that the failure is
182 : > unrelated to your change, please leave a comment on the pull
183 : > request explaining why you believe the failure is unrelated. A
184 : > maintainer will re-run the status check for you. If we conclude
185 : > that the failure was a false positive, then we will open an issue
186 : > to track that problem with our status check suite.
187 :
188 : Only those status check failures that occur in the containerized build
189 : environment are your responsibility to fix. If you encounter an issue with a
190 : status check that runs in an environment that you do not have access to, e.g.
191 : on macOS or on a supercomputer, please notify
192 : `@sxs-collaboration/spectre-core-devs`. They will refer the issue to a person
193 : who has access to that environment. Unless requested by the reviewers, the PR
194 : will not be held up until the issue is resolved.
195 :
196 : While the prerequisites above must be satisfied prior to having your
197 : pull request reviewed, the reviewers may ask you to complete
198 : additional design work, tests, or other changes before your pull
199 : request can be ultimately accepted.
200 :
201 : ## How SpECTRE pull request reviews are conducted {#pull-request-reviews}
202 :
203 : > Note that these are guidelines and not rigid rules.
204 :
205 : Please make your pull requests as small as reasonably possible, as
206 : smaller pull requests are easier to review. In general, longer pull
207 : requests take longer to review, with the time scaling exponentially
208 : with the number of lines changed. Therefore if your pull request is
209 : too large, we may ask you to break it up into several smaller pull
210 : requests.
211 :
212 : If you would like feedback on a pull request prior to it being ready
213 : for formal review, please open it
214 : [in draft mode](https://github.blog/2019-02-14-introducing-draft-pull-requests/)
215 : and [request reviews][request-reviews] from whomever you wish to get feedback
216 : from. As long as the PR is in draft mode it will not be reviewed, aside from the
217 : feedback requested.
218 :
219 : > Below, days mean business days, so if the time period includes the
220 : > weekend, add two days, and for major holidays add a day.
221 : > Furthermore, most of us are academics, and we occasionally go to
222 : > conferences which may lead to delays in the review process. Also do
223 : > not expect much to happen between December 20th and January 3rd.
224 :
225 : Most pull requests submitted to SpECTRE will be reviewed in the
226 : following manner:
227 :
228 : - Within two days, one of the [SpECTRE core developers](#core-developers) will
229 : either review the PR or assign reviewers. If this has not happened after two
230 : days, please [request a review][request-reviews]
231 : and select the `@sxs-collaboration/spectre-core-devs` team. Also feel free to
232 : [ping the core developers][github-ping] (e.g
233 : `@sxs-collaboration/spectre-core-devs please assign reviewers`) if they fail
234 : to respond in a timely manner.
235 : - Assigned reviewers should either confirm that they are able to review (by
236 : reviewing or providing a reasonable timeframe for their review) or decline
237 : within two days so another reviewer can be assigned.
238 : - Anyone is welcome to self-assign themselves as a reviewer.
239 : - Assigned reviewers should submit their review in as timely a manner
240 : as possible.
241 : - Anyone can request changes within either the first two days of the
242 : pull request, or within a day after the initial reviews of the
243 : assigned reviewers. After this period, only the assigned reviewers
244 : can request changes, unless someone believes the code is wrong.
245 : Non-reviewers are allowed to make comments, which the pull request
246 : author is encouraged to address. Alternatively the author can
247 : create an issue with the suggested changes, assigned to themselves,
248 : which would then be addressed in a subsequent pull request by the
249 : author.
250 : - If any requested change is unclear to the author, they should ping
251 : the reviewer and ask for clarification. Authors and reviewers are
252 : encouraged to talk to one another (in person, via Google hangout, or
253 : some other verbal method if possible) to resolve any issues.
254 : - Reviewers are encouraged to [ping others \@GITHUB_USERNAME][github-ping] to
255 : get opinions on code they are unsure about.
256 : - It is permissible to have a group code review led by one of the
257 : reviewers. The reviewer should comment on who was present at the
258 : group review.
259 : - If necessary, pull requests can also be discussed at one of the
260 : weekly SpECTRE meetings.
261 : - If changes are requested, the author should fix all of them in one
262 : or more fixup commits (where the first line of the commit message
263 : should begin with `fixup`) and push them to the PR branch.
264 : Fixup commits make reviews significantly faster because the reviewers don't
265 : have to review the full PR again, but only the parts that changed.
266 : - After pushing fixup commits, the author should
267 : [re-request reviews from the reviewers][request-reviews].
268 : They can also ping the reviewers that the pull request is updated.
269 : - Once a pull request is updated, the reviewers should either request further
270 : changes or approve the PR.
271 : - Once all reviewers have approved the PR or given the okay to squash, the
272 : author should [rebase on develop and then squash their
273 : commits](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History)
274 : into one or more self-contained commits (such that the code will
275 : compile and pass all tests after each commit). The squashed commits
276 : will need to be force pushed.
277 : - Once all reviewers have approved the pull request, someone should
278 : ping the `@sxs-collaboration/spectre-core-devs`. If one of the
279 : original reviewers is a core developer, this is not necessary and
280 : the core developer can merge the pull request.
281 : - One of the core developers will perform a final cursory review,
282 : requesting changes only for major problems, and commenting on other
283 : possible changes.
284 : - The pull request author should address all final requested changes
285 : and may either also fix final suggested changes, or create an issue
286 : with the suggested changes, which will be addressed in a subsequent
287 : pull request by the author.
288 : - The SpECTRE core developer will merge the pull request once all comments have
289 : been addressed, all reviewers have approved the PR, the code passes CI and all
290 : pull requests the pull request depends on have been merged. When approvals are
291 : dismissed by minor changes, such as rebasing, squashing fixup commits or
292 : adding a missing include, the core developer may merge the PR without waiting
293 : for all reviewers to re-approve the PR.
294 :
295 : In addition to the guidelines above, we apply the following exceptions based on
296 : the type of change:
297 :
298 : - Critical bug fixes (i.e. the code is broken) can be merged after two expedited
299 : reviews by SpECTRE core developers. If necessary, an issue can be created if
300 : further changes are desired.
301 : - PRs that add documentation don't need to be perfect, since having some
302 : docs is better than having none. Reviewers should approve after one or at most
303 : two rounds of review and allow further minor changes to be done in follow-ups.
304 : - "Small" or "trivial" PRs can be merged immediately by core developers.
305 : Examples for such PRs are:
306 :
307 : - Fixing typos in documentation or adding (small amounts of) documentation
308 : - Fixing missing includes or missing linked libraries
309 : - Adding Python bindings
310 : - Formatting code
311 : - Refactoring, renaming or moving files with no change in functionality
312 : (unless potentially controversial)
313 :
314 : It is the reviewing core developer's responsibility to decide if the PR is
315 : "small" or "trivial" enough to merge immediately. If they are unsure, they
316 : should fall back to the usual procedure of giving people two days to assign
317 : themselves as reviewers and/or reach out to whoever they think might want to
318 : review the PR.
319 : - Pull requests that are designated `new design` are expected to have a longer
320 : review period, including discussions during weekly SpECTRE meetings. SpECTRE
321 : core developers will provide reasonable review deadlines once the new design
322 : is finalized.
323 :
324 : ### Git Commit Message Guidelines {#git-commit-messages}
325 :
326 : * Use the present tense ("Add feature" not "Added feature")
327 : * Use the imperative mood ("Move cursor to..." not "Moves cursor to...")
328 : * Limit the first line to 72 characters or less
329 : * If needed, a blank second line followed by a more complete description
330 :
331 : ### SpECTRE core developers {#core-developers}
332 :
333 : SpECTRE core developers are people who are very familiar with the
334 : entire code, comfortable with modern C++, and willing to take the
335 : responsibility of overseeing the code as a whole.
336 : [Current SpECTRE core
337 : developers](https://github.com/orgs/sxs-collaboration/teams/spectre-core-devs)
338 : can be pinged on GitHub at `@sxs-collaboration/spectre-core-devs`. It
339 : is expected that as more contributors become familiar with SpECTRE,
340 : additional people will be added to the list of core developers.
341 :
342 :
343 : [good-first-issue]:https://github.com/sxs-collaboration/spectre/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22+sort%3Acomments-desc
344 : [help-wanted]:https://github.com/sxs-collaboration/spectre/issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22+sort%3Acomments-desc
345 : [github-ping]:https://github.blog/2011-03-23-mention-somebody-they-re-notified/
346 : [request-reviews]:https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review
|
