Skip to content

Commit a995a35

Browse files
Merge pull request #1047 from r-lib/update_contributing
2 parents 4c1cb51 + 9365f14 commit a995a35

File tree

2 files changed

+100
-96
lines changed

2 files changed

+100
-96
lines changed

CONTRIBUTING.md

Lines changed: 99 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,75 @@
1-
## Intro
1+
# Contributing to {styler}
2+
3+
## Introduction
24

35
This project follows the contributing recommendations outlined by [saamwerk](https://lorenzwalthert.github.io/saamwerk/).
46
In particular, issues labelled with `Status: Postponed` are closed even if they
57
are not resolved.
68

79
## Contributing code
810

9-
* Only open a PR when your idea was approved of by a contributor in an issue.
10-
* Add a bullet to NEWS.md referencing the PR, also following the guide lines in
11-
[tidyverse style guide](http://style.tidyverse.org), as for your code
12-
contributions.
13-
* Make sure your commit pass the pre-commit hooks in this repo. See the
14-
`{precommit}` [README.md](https://github.com/lorenzwalthert/precommit)
15-
on how to install the pre-commit framework and the R package on your system and
16-
then run `precommit::use_precommit()` to make sure the hooks are activated
17-
in your local styler clone. If you skip a hook, describe why in the PR.
18-
11+
* Open a PR only when your idea was approved of by a contributor in an issue.
12+
* Follow guidelines in [tidyverse style guide](http://style.tidyverse.org) for
13+
your code contributions.
14+
* Make sure your commit pass the pre-commit hooks in this repo. See the
15+
`{precommit}` [README.md](https://github.com/lorenzwalthert/precommit)
16+
on how to install the pre-commit framework and the R package on your system and
17+
then run `precommit::use_precommit()` to make sure the hooks are activated
18+
in your local styler clone. If you skip a hook, describe why in the PR.
1919

2020
## How to dive in and understanding the source code
2121

2222
Read the vignettes. If you are done, come back here.
2323

24-
`devtools::load_all()`
24+
```r
25+
devtools::load_all()
2526

26-
`debug(style_text)`
27+
debug(style_text)
2728

28-
`style_text("call(1, 2 + 1)")`
29+
style_text("call(1, 2 + 1)")
30+
```
2931

3032
Go broad before you go deep. Before going into the very deep layers of function
31-
calls of `style_text()`, try to understand that `style_text()` consists of a few
32-
function calls only.
33-
Go into each of them and try to understand one layer deep. That is, try to
33+
calls of `style_text()`, try to understand that `style_text()` consists of a few
34+
function calls only.
35+
Go into each of them and try to understand one layer deep. That is, try to
3436
understand what `make_transformer()` does by reading the names of the functions
35-
that get called, the name of the objects that are created by assigning the output of
36-
these function calls. Before looking into a functions source code, look at the
37+
that get called, the name of the objects that are created by assigning the output of
38+
these function calls. Before looking into a functions source code, look at the
3739
documentation for that function. All internal important functions
38-
are documented and documentation is available also for unexported objects via
39-
`?` (if you did `devtools::load_all()`.
40-
Then, go into `parse_transform_serialize()` and so on.
41-
42-
To understand the most fundamental operation in styler, the manipulation of the
43-
columns related to spacing and line break information, pick a rule from
44-
`R/rules-*.R`, e.g. `R/rules-spacing`, add a break point to a rule and style a
45-
string where you think this rule will be active. Then, see what happens and how
40+
are documented and documentation is available also for unexported objects via
41+
`?` (if you did `devtools::load_all()`).
42+
Then, go into `parse_transform_serialize()`, and so on.
43+
44+
To understand the most fundamental operation in styler, the manipulation of the
45+
columns related to spacing and line break information, pick a rule from
46+
`R/rules-*.R` (e.g. `R/rules-spacing`), add a break point to a rule, and style a
47+
string where you think this rule will be active. Then, see what happens and how
4648
this rule is applied on each level of nesting.
4749

4850
## Static code analysis
4951

50-
There are multiple packages that can be used to analyze a code base:
52+
There are multiple packages that can be used to analyze a code base:
5153

5254
* [gitsum](https://github.com/lorenzwalthert/gitsum): Parses and summarises git
53-
repository history.
54-
* [parsesum](https://github.com/lorenzwalthert/parsesum): Analyses source code
55-
through parsing.
55+
repository history.
56+
* [parsesum](https://github.com/lorenzwalthert/parsesum): Analyses source code
57+
through parsing.
5658

5759
Check out the links above to see how the tools listed could help you
5860
understanding styler.
5961

6062
## Project setup
6163

62-
* The package is developed with the devtools suite, which includes roxgen2 for
63-
documentation, testthat for unit testing, pkgdown for html documentation.
64-
* Continuous integration is done with the tic (tasks integrating continuously)
65-
package.
66-
* A key development principle of styler is to separate infrastructure from
67-
style guide. Hence, whenever possible, transformer functions should be
68-
adapted, not the infrastructure should be changed for a specific style guide.
69-
* styler was created in 2017 by Kirill Müller, then turned from a
64+
* The package is developed with {devtools} suite, which includes {roxgen2} for
65+
documentation, {testthat} for unit testing, {pkgdown} for HTML documentation.
66+
* Continuous integration uses github-actions.
67+
* A key development principle of styler is to separate infrastructure from
68+
style guide. Hence, whenever possible, transformer functions should be
69+
adapted, instead of changing the infrastructure for a specific style guide.
70+
* {styler} was created in 2017 by Kirill Müller. It was then turned from a
7071
proof-of-concept into a ready-for-production tool as part of GSOC 2017 with
71-
Kirill Müller and Yihui Xie as mentors and Lorenz Walthert as student.
72-
72+
Kirill Müller and Yihui Xie as mentors and Lorenz Walthert as student.
7373

7474
## File Structure
7575

@@ -79,128 +79,131 @@ The source code is organized as follows:
7979
| -------------: |:-----------------------------------------------------------|
8080
| addins.R | ui and helpers for the Addins of styler. |
8181
| communicate.R | function to communicate to the user via the console. |
82-
| compat-dplyr.R | compatibility functions. Since styler does not depend on dplyr, we define the dplyr functions ourself.|
83-
| compat-tidyr.R | compatibility functions. Since styler does not depend on tidy, we define the tidyr functions ourself.|
82+
| compat-dplyr.R | compatibility functions. Since styler does not depend on dplyr, we define the dplyr functions ourself.|
83+
| compat-tidyr.R | compatibility functions. Since styler does not depend on tidy, we define the tidyr functions ourself.|
8484
| expr-is.R | Functions to check whether an expression matches a predicate (e.g. whether it *is* a function call, a curly brace expression etc.). |
85-
| indent.R | Computation of whether indention is needed (needs_indention()), if so which indices are indented and how indention is it is triggered. |
86-
| initialize.R | initializer called with the visitor at each nest. |
85+
| indent.R | Computation of whether indention is needed (`needs_indention()`), if so which indices are indented and how indention is it is triggered. |
86+
| initialize.R | initializer called with the visitor at each nest. |
8787
| nest.R | converting from a text representation into a flat and then into a nested parse table representation. |
8888
| nested-to-tree.R | utilities to create a tree representation from text (after text was converted into a nested parse table). |
89-
| parse.R | parse text into parse table, minor token manipulation, verification of parsed objects. |
89+
| parse.R | parse text into parse table, minor token manipulation, verification of parsed objects. |
9090
| reindent.R | Deals with token-dependent indention and re-indention, opposed to indent.R where all indention is token independent (i.e. a brace just adds one level of indention, whereas in function declaration headers (if mutli-line), indention depends on token position of "function"). |
9191
| relevel.R | Reorganizing the nested parse table, namely relocates expressions on both sides of "%>%" to the same nest. |
92-
| rules-line-break.R, rules-other.R, rules-replacement.R, rules-spacing.R | transformer rules |
93-
| serialize.R | converts flattened parse table into text representation. Complement operation to the functions in nest.R |
94-
| set-assert-args.R | Assertion and setting of arguments. |
95-
| style-guides.R | How to create style guide objects from transformers. |
92+
| rules-line-break.R, rules-other.R, rules-replacement.R, rules-spacing.R | transformer rules |
93+
| serialize.R | converts flattened parse table into text representation. Complement operation to the functions in nest.R |
94+
| set-assert-args.R | Assertion and setting of arguments. |
95+
| style-guides.R | How to create style guide objects from transformers. |
9696
|styler.R | General package information. |
97-
| testing.R | function used for testing. |
98-
| token-create.R | Utilities for creating tokens, mostly to insert braces around mutli-line if statements. |
97+
| testing.R | function used for testing. |
98+
| token-create.R | Utilities for creating tokens, mostly to insert braces around mutli-line if statements. |
9999
| token-define.R | Defines which tokens belong to which group. |
100-
| transform-code.R, transform-files.R | Transformation of code for APIs that manipulate files (e.g. style_file()). |
101-
| ui.R | User interaces. Top-level functions for styling. |
102-
| unindent.R | Certain tokens cause unindention, e.g. closing braces. |
100+
| transform-code.R, transform-files.R | Transformation of code for APIs that manipulate files (e.g. `style_file()`). |
101+
| ui.R | User interaces. Top-level functions for styling. |
102+
| unindent.R | Certain tokens cause unindention, e.g. closing braces. |
103103
| utils.R | low-level general purpose utilities. |
104-
| vertical.R | S3 class for pretty printing of styled code. |
105-
| visit.R | Functions that apply functions to each level of nesting, either inside out or outside in. |
104+
| vertical.R | S3 class for pretty printing of styled code. |
105+
| visit.R | Functions that apply functions to each level of nesting, either inside out or outside in. |
106106
| zzz.R | backport imports. |
107107

108108
## Obtaining contextual information
109109

110110
You may have problems understanding some code because documentation is minimal,
111111
some code / functions seem to solve problems you don't understand or handle
112-
cases that seem unreasonable or otherwise incomprehensible. You can resort to
112+
cases that seem unreasonable or otherwise incomprehensible. You can resort to
113113
the following strategies:
114114

115115
* Use full-text search to see where functions are defined or called and how
116-
different parts of styler depend on it.
117-
* Use `$git blame` to see where changes were introduced. Look at the commit
116+
different parts of {styler} depend on it.
117+
* Use `$ git blame` to see where changes were introduced. Look at the commit
118118
message, check changes that were made to the code in the same commit. If you
119119
are using the GUI of GitHub, you can easily obtain more contextual information
120-
such as the pull request with which a change was introduced. Often
121-
functionality was introduced with testing. So, you can easily see which new
122-
tests are related to the new functionality. You can remove the changes in the
120+
such as the pull request with which a change was introduced. Often,
121+
functionality was introduced with testing. So, you can easily see which new
122+
tests are related to the new functionality. You can remove the changes in the
123123
source code and re-run the tests and see what fails and why.
124-
* Search Issues and Pull Requests on GitHub with the full text search. Make
124+
* Search Issues and Pull Requests on GitHub with the full text search. Make
125125
sure you also search for closed Issues and PRs.
126126

127-
128127
## High-level conventions
129128

130-
* The project follows a highly functional approach. This means that
129+
* The project follows a highly functional approach. This means that
131130
functionality should be capsuled into functions, even if they are only called
132131
once. This makes abstraction from the code easier, reduces the number of lines
133-
for each function declaration considerably and makes it easier for people not
134-
familiar with the code base to dive into it.
132+
for each function declaration considerably, and makes it easier for people not
133+
familiar with the codebase to dive into it.
135134
* All internal functions (except if they are 100% self-explanatory) are to be
136135
documented.
137136
* New functionality (e.g. in terms of styling rules) needs to be unit tested. If
138-
the new functionality changes how code is to be styled, the infrastructure
137+
the new functionality changes how code is to be styled, the infrastructure
139138
with `test_collection()` should be used.
140139
* Cases that are not yet formatted correctly can be labelled with a `FIXME`.
141140
* GitHub is the platform where communication about source code happens. We
142-
refrain from adding extensive in-line code comments. One can use `git blame`
141+
refrain from adding extensive in-line code comments. One can use `$ git blame`
143142
to track when changes were introduced and find the corresponding pull request
144-
and associated issues to understand the thought process that lead to a change
145-
in the source code. This also implies that issues and / or pull request
146-
contain verbose explanation of problems and solutions provided.
143+
and associated issues to understand the thought process that lead to a change
144+
in the source code. This also implies that issues and / or pull request
145+
contain verbose explanation of problems and solutions provided.
147146

148147
## Low-level coventions
149148

150-
This project follows the [tidyverse style guide](http://style.tidyverse.org).
149+
This project follows the [tidyverse style guide](http://style.tidyverse.org).
151150
If we refer to specific variables / values etc. in the following sections, you
152-
can use RStudio's full text search to find where
151+
can use RStudio's full text search to find where
153152
`remove_line_break_before_round_closing_after_curly()` is declared or called.
154153

155-
156154
### Files
157155

158156
* File names only contain alphanumeric characters and dashes.
159-
* Files are named according to topics / contexts, not according to functions
157+
* Files are named according to topics / contexts, not according to functions
160158
that live in these files.
161159

162160
### Functions
163161

164-
* Function names should be verbs. No abbreviations should be used, we don't
162+
* Function names should be verbs. No abbreviations should be used, we don't
165163
care if function names are particularly long. For example, there is a
166164
function with the name `remove_line_break_before_round_closing_after_curly()`.
167-
* only very low-level functions or functions that don't fit in any other file
165+
* Only very low-level functions or functions that don't fit in any other file
168166
go to `utils.R`.
169167

170168
### Control Flow
171169

172-
* Conditional statements should always evaluate to `TRUE` or `FALSE`, i.e. we
173-
don't encourage `if (length(x))` but rather `if (length(x) > 0L)`.
174-
* We avoid loops whenever possible and use functions like `purrr::map()` and
175-
friends when possible and prefer them over R base counterparts like
170+
* Conditional statements should always evaluate to `TRUE` or `FALSE`, i.e. we
171+
don't encourage `if (length(x))`, but rather `if (length(x) > 0L)`.
172+
* We avoid loops whenever possible and use functions like `purrr::map()` and
173+
friends when possible and prefer them over R base counterparts like
176174
`base::lapply()`.
177175

178176
### Boolean Values
179177

180178
Functions that return Boolean values or variables that hold Boolean values are
181-
often prefixed with `is` or `has`. For example, `is_rmd_file(path)` is a
179+
to be prefixed with `is` or `has`. For example, `is_rmd_file(path)` is a
182180
function that returns `TRUE` if `path` is the path to a `.Rmd` file and `FALSE`
183-
otherwise.
181+
otherwise.
184182

185183
### Vectors with indices
186184

187-
Vectors that hold indices are often suffixed with `idx`. `else_idx` for example
185+
Vectors that hold indices are often suffixed with `idx`. For example, `else_idx`
188186
indicates for every row in a parse table whether it contains an `else` token.
189187

190188
### Closures
191189

192-
The use of closures is discouraged. We prefer to prefill a template function
190+
The use of closures is discouraged. We prefer to prefill a template function
193191
with `purrr::partial()`.
194192

195193
## Testing
196194

197-
We have a testing framework powered by `test_collection()`.
195+
We have a testing framework powered by `test_collection()`.
198196
Essentially, there is an \*-in.R file and a \*-out.R file. The \*-in.R file is the
199-
input that is transformed and - if it matches the *-out.R file, the test has
200-
passed. You can create an \*-in.R file, run `devtools::test(f = "[your file]")`
201-
and an \*-out.R file is generated. If the file matches your expectation,
202-
you can commit it. Note that files are overwritten and version control should be
203-
used to track failed tests.
204-
The files are placed in `tests/testthat` under the category they fit.
205-
Please have a look at the documentation for `test_collection()` and see other
206-
unit tests. Let me know if there is anything unclear about this.
197+
input that is transformed and - if it matches the *-out.R file, the test will
198+
pass. You can create an \*-in.R file, run `devtools::test(f = "[your file]")`
199+
and an \*-out.R file is generated. If the file matches your expectation,
200+
you can commit it. **Note that files are overwritten and version control should be
201+
used to track failed tests.**
202+
The files are placed in `tests/testthat` under the category they fit.
203+
Please have a look at the documentation for `test_collection()` and see other
204+
unit tests.
205+
206+
## Feedback
207+
208+
Please open an issue if something is unclear so that we can improve the
209+
contributing guidelines.

inst/WORDLIST

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ chnages
2121
ci
2222
cli
2323
CMD
24+
codebase
2425
codecov
2526
coercible
2627
coercions

0 commit comments

Comments
 (0)