admin/topgrade
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: Improve CONTRIBUTING.md (#1420)

Browse Source
This commit is contained in:
Gideon
2025-11-02 16:40:11 +01:00
committed by GitHub
parent f669de8272
commit bafa15c8f1
1 changed files with 50 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
CONTRIBUTING.md
View File

@@ -7,13 +7,24 @@ We welcome and encourage contributions of all kinds, such as:
2. Documentation improvements
3. Code (PR or PR Review)
Please follow the [Karma Runner guidelines](http://karma-runner.github.io/6.2/dev/git-commit-msg.html)
for commit messages.
### General guidelines
## Adding a new `step`
**Please use [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) for your PR title**.
In `topgrade`'s term, package manager is called `step`.
To add a new `step` to `topgrade`:
We use [pre-commit](https://github.com/pre-commit/pre-commit). It runs in CI, but you can optionally install the hook
locally with `pre-commit install`. If you don't want to use pre-commit, make sure the following pass before submitting
your PR:
```shell
$ cargo fmt
$ cargo clippy
$ cargo test
```
### Adding a new step
In `topgrade`'s terms, a package manager (or something else that can be upgraded) is called a step.
To add a new step to `topgrade`:
1. Add a new variant to
[`enum Step`](https://github.com/topgrade-rs/topgrade/blob/main/src/step.rs)
@@ -33,8 +44,9 @@ To add a new `step` to `topgrade`:
You need to find the appropriate location where this update function goes, it should be
a file under [`src/steps`](https://github.com/topgrade-rs/topgrade/tree/main/src/steps),
the file names are self-explanatory, for example, `step`s related to `zsh` are
placed in [`steps/zsh.rs`](https://github.com/topgrade-rs/topgrade/blob/main/src/steps/zsh.rs).
the file names are self-explanatory, for example, steps related to `zsh` are
placed in [`steps/zsh.rs`](https://github.com/topgrade-rs/topgrade/blob/main/src/steps/zsh.rs), and steps that run on
Linux only are placed in [`steps/linux.rs`](https://github.com/topgrade-rs/topgrade/blob/main/src/steps/linux.rs).
Then you implement the update function, and put it in the file where it belongs.
@@ -53,19 +65,17 @@ To add a new `step` to `topgrade`:
}
```
Such a update function would be conventionally named `run_xxx()`, where `xxx`
is the name of the new step, and it should take a argument of type
`&ExecutionContext`, this is adequate for most cases unless some extra stuff is
needed (You can find some examples where extra arguments are needed
[here](https://github.com/topgrade-rs/topgrade/blob/7e48c5dedcfd5d0124bb9f39079a03e27ed23886/src/main.rs#L201-L219)).
Such an update function would be conventionally named `run_xxx()`, where `xxx`
is the name of the new step, and it should take an argument of type
`&ExecutionContext`.
Update function would usually do 3 things:
The update function should usually do 3 things:
1. Check if the step is installed
2. Output the Separator
3. Invoke the step
2. Output the separator
3. Execute commands
Still, this is sufficient for most tools, but you may need some extra stuff
with complicated `step`.
This is sufficient for most tools, but you may need some extra stuff
for complicated steps.
3. Add a match arm to `Step::run()`
@@ -74,7 +84,7 @@ To add a new `step` to `topgrade`:
```
We use [conditional compilation](https://doc.rust-lang.org/reference/conditional-compilation.html)
to separate the steps, for example, for steps that are Linux-only, it goes
to separate the steps. For example, for steps that are Linux-only, it goes
like this:
```rust
@@ -89,11 +99,11 @@ To add a new `step` to `topgrade`:
```rust
steps.push(Xxx)
```
Try to keep the conditional compilation the same as in the above step 3.
Keep the conditional compilation the same as in the above step 3.
Congrats, you just added a new `step` :)
Congrats, you just added a new step :)
## Modification to the configuration entries
### Modification to the configuration entries
If your PR has the configuration options
(in [`src/config.rs`](https://github.com/topgrade-rs/topgrade/blob/main/src/config.rs))
@@ -106,10 +116,10 @@ Be sure to apply your changes to
[`config.example.toml`](https://github.com/topgrade-rs/topgrade/blob/main/config.example.toml),
and have some basic documentations guiding user how to use these options.
## Breaking changes
### Breaking changes
If your PR introduces a breaking change, document it in [`BREAKINGCHANGES_dev.md`][bc_dev],
it should be written in Markdown and wrapped at 80, for example:
If your PR introduces a breaking change, document it in [`BREAKINGCHANGES_dev.md`][bc_dev].
It should be written in Markdown and wrapped at 80, for example:
```md
1. The configuration location has been updated to x.
@@ -121,25 +131,12 @@ it should be written in Markdown and wrapped at 80, for example:
[bc_dev]: https://github.com/topgrade-rs/topgrade/blob/main/BREAKINGCHANGES_dev.md
## Before you submit your PR
Make sure your patch passes the following tests on your host:
```shell
$ cargo build
$ cargo fmt
$ cargo clippy
$ cargo test
```
Don't worry about other platforms, we have most of them covered in our CI.
## I18n
### I18n
If your PR introduces user-facing messages, we need to ensure they are translated.
Please add the translations to [`locales/app.yml`][app_yml]. For simple messages
without arguments (e.g., "hello world"), we can simply translate them according
(Tip: ChatGPT or similar LLMs is good at translation). If a message contains
(Tip: LLMs are good at translation). If a message contains
arguments, e.g., "hello <NAME>", please follow this convention:
```yml
@@ -152,24 +149,22 @@ a preceding `%` when used in translations.
[app_yml]: https://github.com/topgrade-rs/topgrade/blob/main/locales/app.yml
## Some tips
### Locales
1. Locale
Some steps respect locale, which means their output can be in language other
than English. In those cases, we cannot rely on the output of a command.
Some `step` respects locale, which means their output can be in language other
than English, we should not do check on it.
For example, one may want to check if a tool works by doing this:
For example, one may want to check if a tool works by doing this:
```rust
let output = Command::new("xxx").arg("--help").output().unwrap();
let stdout = from_utf8(output.stdout).expect("Assume it is UTF-8 encoded");
```rust
let output = Command::new("xxx").arg("--help").output().unwrap();
let stdout = from_utf8(output.stdout).expect("Assume it is UTF-8 encoded");
if stdout.contains("help") {
// xxx works
}
```
if stdout.contains("help") {
// xxx works
}
```
If `xxx` respects locale, then the above code should work on English system,
on a system that does not use English, e.g., it uses Chinese, that `"help"` may be
translated to `"帮助"`, and the above code won't work.
If `xxx` respects locale, then the above code should work on English system,
on a system that does not use English, e.g., it uses Chinese, that `"help"` may be
translated to `"帮助"`, and the above code won't work.