From d79e1662edbca4ef9707d3ea857e6fda03d172fe Mon Sep 17 00:00:00 2001 From: Paul Hansen Date: Sat, 12 Aug 2023 17:24:09 -0500 Subject: [PATCH 01/10] Initial CONTRIBUTING.md --- CONTRIBUTING.md | 71 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..9b712169 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,71 @@ +# Contributing to AccessKit + +## Making a Pull Request + +### Cargo.lock File + +AccessKit intentionally includes the `Cargo.lock` file in our git repository. +This is mainly due to our bindings to other languages. + +Usually you should not run cargo update when creating a pull request even when adding/changing a dependency. +Simply building the library should update the `Cargo.lock` file with the minimal changes needed. + +> [!NOTE] +> This is not normal / best practice for most libraries. +> If you are writing your own library, your `Cargo.lock` file is not used when the library is included in another project. +> This means your CI will be testing with an out of date set of dependencies compared to your users, missing potential problems. +> See the [official documentation](https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries) for more information. + +### CHANGELOG.md + +Our CHANGELOG.md files are auto generated using [Release Please](https://github.com/googleapis/release-please) and should not be edited manually. + +To control what is in the CHANGELOG.md for your change, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) when writing your commit messages. + +#### Example Commit Messages +Taken from [Conventional Commits documentation](https://www.conventionalcommits.org/en/v1.0.0/#summary): +> feat: allow provided config object to extend other configs +> +> BREAKING CHANGE: `extends` key in config file is now used for extending other config files + +> fix: prevent racing of requests +> +> Introduce a request id and a reference to latest request. Dismiss +incoming responses other than from latest request. + +### Testing Locally + +We have some platform specific tests that are not run with `cargo test` from the project root by default. + +1. Run test that work on all platforms + ``` shell + cargo test + ``` +2. Run platform specific tests. + + Run the appropriate command for your platform + ``` shell + cargo test -p accesskit_windows + cargo test -p accesskit_unix + cargo test -p accesskit_macos + ``` + Not all platforms have tests at this time, but they may in the future. + +> [!WARNING] +> **Windows**: Some tests may fail if the created window looses focus while testing. This can also happen when using the terminal built into your IDE. Try running them from your native terminal if you get failures. + +### Common issues + +#### CI Errors + +If you get an error in the CI step `Run cmake --build build` that includes near the end of the error: +``` +ERROR: Couldn't generate bindings for /home/runner/work/accesskit/accesskit/bindings/c. +``` +Somewhere in the big block of text in the error message it also says: +``` +unknown feature `proc_macro_span_shrink` +``` + +This is likely not an issue with your PR and just means `proc-macro2` needs to be updated in our Cargo.lock file. +You probably want to make this change in a separate PR as `proc-macro2` can update often and your PR could become out of date if it takes a while to be reviewed. \ No newline at end of file From 068ed5a302455e71e4178bd3b628d7210b1223f6 Mon Sep 17 00:00:00 2001 From: Paul Hansen Date: Sat, 12 Aug 2023 17:54:23 -0500 Subject: [PATCH 02/10] Remove lines about Cargo.lock that I'm not sure are true --- CONTRIBUTING.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9b712169..b724fa14 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,8 +12,6 @@ Simply building the library should update the `Cargo.lock` file with the minimal > [!NOTE] > This is not normal / best practice for most libraries. -> If you are writing your own library, your `Cargo.lock` file is not used when the library is included in another project. -> This means your CI will be testing with an out of date set of dependencies compared to your users, missing potential problems. > See the [official documentation](https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries) for more information. ### CHANGELOG.md From f160d8a00f32af3f41b83ab99834a9f62c9ff6d5 Mon Sep 17 00:00:00 2001 From: Paul Hansen Date: Mon, 14 Aug 2023 13:54:07 -0500 Subject: [PATCH 03/10] Remove common issues section for now --- CONTRIBUTING.md | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b724fa14..62334b5e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,19 +51,3 @@ We have some platform specific tests that are not run with `cargo test` from the > [!WARNING] > **Windows**: Some tests may fail if the created window looses focus while testing. This can also happen when using the terminal built into your IDE. Try running them from your native terminal if you get failures. - -### Common issues - -#### CI Errors - -If you get an error in the CI step `Run cmake --build build` that includes near the end of the error: -``` -ERROR: Couldn't generate bindings for /home/runner/work/accesskit/accesskit/bindings/c. -``` -Somewhere in the big block of text in the error message it also says: -``` -unknown feature `proc_macro_span_shrink` -``` - -This is likely not an issue with your PR and just means `proc-macro2` needs to be updated in our Cargo.lock file. -You probably want to make this change in a separate PR as `proc-macro2` can update often and your PR could become out of date if it takes a while to be reviewed. \ No newline at end of file From 4ea0cb6473ba9fcbc94c298276b82f050d44b747 Mon Sep 17 00:00:00 2001 From: Paul Hansen Date: Tue, 15 Aug 2023 10:59:45 -0500 Subject: [PATCH 04/10] Add a link to CONTRIBUTING in the README --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index fd35a41a..c4cf1ec7 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,10 @@ While many languages can use a C API, we also plan to provide libraries that mak We realize that most developers who might use AccessKit are not experts in accessibility. So this project will need to include comprehensive documentation, including a conceptual overview for developers that are learning about accessibility for the first time. +## Contributing + +Contributions to AccessKit are welcome. Please see [CONTRIBUTING.md](./CONTRIBUTING.md). + ## License AccessKit is licensed under the [Apache License, Version 2.0](LICENSE-APACHE) or the [MIT license](LICENSE-MIT), at your option. From e0d0fb890d48a2429ae0c524b4ab4dc1b3f71459 Mon Sep 17 00:00:00 2001 From: Paul Hansen Date: Tue, 15 Aug 2023 12:12:31 -0500 Subject: [PATCH 05/10] Tweak to say PR title matters for changelogs not commit title --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 62334b5e..076a108e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,7 @@ Simply building the library should update the `Cargo.lock` file with the minimal Our CHANGELOG.md files are auto generated using [Release Please](https://github.com/googleapis/release-please) and should not be edited manually. -To control what is in the CHANGELOG.md for your change, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) when writing your commit messages. +To control what is in the CHANGELOG.md for your change, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) when writing your PR title. #### Example Commit Messages Taken from [Conventional Commits documentation](https://www.conventionalcommits.org/en/v1.0.0/#summary): From 3d0d616172ea152e846b63fa2eccfc85ffb5b58a Mon Sep 17 00:00:00 2001 From: DataTriny Date: Sun, 20 Aug 2023 15:05:17 +0200 Subject: [PATCH 06/10] Add a chapter about issues --- CONTRIBUTING.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 076a108e..a6c415af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,12 @@ # Contributing to AccessKit +## Reporting an issue + +When reporting an issue, in order to help the maintainers understand what the problem is, please make your description of the issue as detailed as possible: + +- if it is a bug, please provide clear explanation of what happens, what should happen, and how to reproduce the issue, ideally by providing a minimal program exhibiting the problem +- if it is a feature request, please provide a clear argumentation about why you believe this feature should be supported by AccessKit + ## Making a Pull Request ### Cargo.lock File From 4e80b60893c589092827b9fc131d71f11311c628 Mon Sep 17 00:00:00 2001 From: DataTriny Date: Sun, 20 Aug 2023 15:12:13 +0200 Subject: [PATCH 07/10] Update the chapter about Cargo.lock --- CONTRIBUTING.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a6c415af..72fe66a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,15 +11,11 @@ When reporting an issue, in order to help the maintainers understand what the pr ### Cargo.lock File -AccessKit intentionally includes the `Cargo.lock` file in our git repository. -This is mainly due to our bindings to other languages. +AccessKit intentionally includes the `Cargo.lock` file in the git repository. -Usually you should not run cargo update when creating a pull request even when adding/changing a dependency. -Simply building the library should update the `Cargo.lock` file with the minimal changes needed. - -> [!NOTE] -> This is not normal / best practice for most libraries. -> See the [official documentation](https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries) for more information. +You should not run `cargo update` when creating a pull request even when adding or changing a dependency. +Simply building the library will update the `Cargo.lock` file with the minimal changes needed. +Remember to commit these changes as part of your pull request. ### CHANGELOG.md From 4424f572e3685d1fcd874af21a735534ee114227 Mon Sep 17 00:00:00 2001 From: DataTriny Date: Sun, 20 Aug 2023 15:23:56 +0200 Subject: [PATCH 08/10] Update the Changelog chapter --- CONTRIBUTING.md | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 72fe66a6..a4b2bf0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,20 +19,10 @@ Remember to commit these changes as part of your pull request. ### CHANGELOG.md -Our CHANGELOG.md files are auto generated using [Release Please](https://github.com/googleapis/release-please) and should not be edited manually. +Our `CHANGELOG.md` files are auto generated using [Release Please](https://github.com/googleapis/release-please) and should not be edited manually. -To control what is in the CHANGELOG.md for your change, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) when writing your PR title. - -#### Example Commit Messages -Taken from [Conventional Commits documentation](https://www.conventionalcommits.org/en/v1.0.0/#summary): -> feat: allow provided config object to extend other configs -> -> BREAKING CHANGE: `extends` key in config file is now used for extending other config files - -> fix: prevent racing of requests -> -> Introduce a request id and a reference to latest request. Dismiss -incoming responses other than from latest request. +To control how your work will be described in the changelog, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) when writing the title of your pull request. +If you think one line is not enough, mention it in your pull request so that maintainers can update the description of the merge commit. ### Testing Locally From d2383c8766e82c5946d0da9744b9fabadcc2a276 Mon Sep 17 00:00:00 2001 From: DataTriny Date: Sun, 20 Aug 2023 15:39:14 +0200 Subject: [PATCH 09/10] Update the tests chapter --- CONTRIBUTING.md | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a4b2bf0e..dba026da 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,21 +26,18 @@ If you think one line is not enough, mention it in your pull request so that mai ### Testing Locally -We have some platform specific tests that are not run with `cargo test` from the project root by default. +We have platform-specific tests that do not run when doing `cargo test` from the project root directory. -1. Run test that work on all platforms +1. Run cross-platform tests: ``` shell cargo test ``` -2. Run platform specific tests. - - Run the appropriate command for your platform +2. Run platform-specific tests by issuing the appropriate command for your platform: ``` shell - cargo test -p accesskit_windows - cargo test -p accesskit_unix cargo test -p accesskit_macos + cargo test -p accesskit_unix + cargo test -p accesskit_windows ``` - Not all platforms have tests at this time, but they may in the future. > [!WARNING] -> **Windows**: Some tests may fail if the created window looses focus while testing. This can also happen when using the terminal built into your IDE. Try running them from your native terminal if you get failures. +> **Windows**: Some end-to-end tests may fail if the created window loses focus. This can happen when using the terminal built into your IDE. Try running them from Powershell or the Command Prompt instead. From 5e8a77e21c8913a38b51709bcdbecd81063341a4 Mon Sep 17 00:00:00 2001 From: DataTriny Date: Sun, 20 Aug 2023 16:24:38 +0200 Subject: [PATCH 10/10] Add a checklist for pull requests --- CONTRIBUTING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dba026da..d5090927 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,14 @@ When reporting an issue, in order to help the maintainers understand what the pr ## Making a Pull Request +When making a code contribution to AccessKit, before opening your pull request please make sure that: + +- your patch builds with AccessKit's minimal supported rust version - Rust 1.64 +- you added tests where applicable +- you tested your modifications on all impacted platforms (see below) +- you updated any relevant documentation +- you left comments in your code explaining any part that is not straightforward, so that the maintainers and future contributors don't have to guess what your code is supposed to do + ### Cargo.lock File AccessKit intentionally includes the `Cargo.lock` file in the git repository.