[DRAFT] Format the rust-lang/rust repository #21
Conversation
|
|
||
| We add a command `install-fmt-hook` to rustbuild. | ||
| This command will install a pre-commit git-hook. | ||
| This git-hook will check that the formatting in the commit is good. |
There was a problem hiding this comment.
Er, I think it should format the code before committing, not just check that it's good, right?
There was a problem hiding this comment.
Not sure; @matklad requested it so they know best what they wanted. =P
| is perfectly good aside from formatting. For these situations, a | ||
| `@rustbot fmt` command is provided with which the author, a reviewer, | ||
| or a T-release member can simply ask `@rustbot` to run `./x.py fmt` | ||
| on their PR and add a commit for that. |
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't know when we'll have the time to implement this, but it seems reasonable.
There was a problem hiding this comment.
@Mark-Simulacrum The one thing I'm wondering is if @rustbot fmt can add commits to the branch of a PR author or not.
There was a problem hiding this comment.
That's definitely possible, at least in general -- the PR author can untick the relevant checkbox but there's no real reason we need to worry about that. The feature is enabled by default.
| The command formats the repository according to the style. | ||
|
|
||
| 4. Add a command `@rustbot fmt` on GitHub which will add a commit to a | ||
| pull request (PR) such that the applied to PR will pass CI in terms of the style. |
There was a problem hiding this comment.
I would personally prefer for this to instead rebase and amend each commit, running rustfmt on the individual commits. I'm not sure if that'll potentially cause conflicts (that we would need to manually resolve); if so then I guess the additional commit is our best bet.
There was a problem hiding this comment.
Can we add a modifier @rustbot fmt fixup which does that upon request?
There was a problem hiding this comment.
Force-pushing the branch also means if the user wants to update their local copy they can't do git pull (or equivalents) anymore, which will be confusing for contributors with less git experience.
|
|
||
| To facilitate incremental enforcement of the style and to disrupt development | ||
| minimally inside the repository, the `tidy` tool will only enforce the style | ||
| in folders with an `.enforcefmt` file. In such folders, the style will be |
There was a problem hiding this comment.
I would personally prefer a list in tidy itself rather than dozens of these files strewn around the repository, but I don't care too much.
There was a problem hiding this comment.
Implementors prerogative? I was thinking .enforcefmt would be easier for the transition since you can incrementally just "move up" the .enforcefmt file instead of having to fiddle with tidy.
| is perfectly good aside from formatting. For these situations, a | ||
| `@rustbot fmt` command is provided with which the author, a reviewer, | ||
| or a T-release member can simply ask `@rustbot` to run `./x.py fmt` | ||
| on their PR and add a commit for that. |
There was a problem hiding this comment.
I don't know when we'll have the time to implement this, but it seems reasonable.
|
|
||
| We add a command `install-fmt-hook` to rustbuild. | ||
| This command will install a pre-commit git-hook. | ||
| This git-hook will check that the formatting in the commit is good. |
There was a problem hiding this comment.
Er, I think it should format the code before committing, not just check that it's good, right?
|
|
||
| 2. Enforce the style with `src/tools/tidy` in the repository's continuous integration (CI). | ||
|
|
||
| 3. Add a command `fmt` to rustbuild (`./x.py`). |
There was a problem hiding this comment.
Could we make this run as a pre-./x.py build hook (or allow configuring this to be the case)? I'd hate to have to wait for everything to rebuild just because a fmt ran and messed something up really low down.
There was a problem hiding this comment.
An opt-in flag to config.toml sound good? I wouldn't make it the default as it may throw around code I'm working on and throw me off when developing.
| This style is adopted to make the repository feel idiomatic in terms of style. | ||
|
|
||
| However, we make a small modification to the style guide. | ||
| In particular, we enable [`use_small_heuristics = "Max"`][small]. |
There was a problem hiding this comment.
+1
FWIW this, newline_style = "Unix", and edition = "2018" are the only rustfmt.toml sections modifications that Fuchsia uses as well.
Rendered
This is a draft version of an RFC for you to review, before a formal proposal is made for consideration.