From 178c04b59cfc387efb90fbf2460f5171512ebfc4 Mon Sep 17 00:00:00 2001 From: outfoxxed Date: Fri, 13 Mar 2026 00:32:43 -0700 Subject: [PATCH] docs: revise contribution policy and related files --- BUILD.md | 10 +- CONTRIBUTING.md | 247 +++++------------------------------------------- HACKING.md | 226 ++++++++++++++++++++++++++++++++++++++++++++ README.md | 4 +- 4 files changed, 263 insertions(+), 224 deletions(-) create mode 100644 HACKING.md diff --git a/BUILD.md b/BUILD.md index 29aecac..aa04bbd 100644 --- a/BUILD.md +++ b/BUILD.md @@ -67,7 +67,13 @@ Dependencies: `cpptrace` Note: `-DVENDOR_CPPTRACE=ON` can be set to vendor cpptrace using FetchContent. -When using FetchContent, `libunwind` is required, and `libdwarf` can be provided by the package manager or fetched with FetchContent. +When using FetchContent, `libunwind` is required, and `libdwarf` can be provided by the +package manager or fetched with FetchContent. + +*Please ensure binaries have usable symbols.* We do not necessarily need full debuginfo, but +leaving symbols in the binary is extremely helpful. You can check if symbols are useful +by sending a SIGSEGV to the process and ensuring symbols for the quickshell binary are present +in the trace. ### Jemalloc We recommend leaving Jemalloc enabled as it will mask memory fragmentation caused @@ -236,7 +242,7 @@ Only `ninja` builds are tested, but makefiles may work. #### Configuring the build ```sh -$ cmake -GNinja -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo [additional disable flags from above here] +$ cmake -GNinja -B build -DCMAKE_BUILD_TYPE=Release [additional disable flags from above here] ``` Note that features you do not supply dependencies for MUST be disabled with their associated flags diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 39fab13..73e7931 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,235 +1,40 @@ -# Contributing / Development -Instructions for development setup and upstreaming patches. +# Contributing -If you just want to build or package quickshell see [BUILD.md](BUILD.md). +Thank you for taking the time to contribute. +To ensure nobody's time is wasted, please follow the rules below. -## Development +## Acceptable Code Contributions -Install the dependencies listed in [BUILD.md](BUILD.md). -You probably want all of them even if you don't use all of them -to ensure tests work correctly and avoid passing a bunch of configure -flags when you need to wipe the build directory. +- All changes submitted MUST be **fully understood by the submitter**. If you do not know why or how + your change works, do not submit it to be merged. You must be able to explain your reasoning + for every change. -Quickshell also uses `just` for common development command aliases. +- Changes MUST be submitted by a human who will be responsible for them. Changes submitted without + a human in the loop such as automated tooling and AI Agents are **strictly disallowed**. Accounts + responsible for such contribution attempts **will be banned**. -The dependencies are also available as a nix shell or nix flake which we recommend -using with nix-direnv. +- Changes MUST respect Quickshell's license and the license of any source works. Changes including + code from any other works must disclose the source of the code, explain why it was used, and + ensure the license is compatible. -Common aliases: -- `just configure [ [extra cmake args]]` (note that you must specify debug/release to specify extra args) -- `just build` - runs the build, configuring if not configured already. -- `just run [args]` - runs quickshell with the given arguments -- `just clean` - clean up build artifacts. `just clean build` is somewhat common. +- Changes must follow the guidelines outlined in [HACKING.md](HACKING.md) for style and substance. -### Formatting -All contributions should be formatted similarly to what already exists. -Group related functionality together. +- Changes must stand on their own as a unit. Do not make multiple unrelated changes in one PR. + Changes depending on prior merges should be marked as a draft. -Run the formatter using `just fmt`. -If the results look stupid, fix the clang-format file if possible, -or disable clang-format in the affected area -using `// clang-format off` and `// clang-format on`. +## Acceptable Non-code Contributions -#### Style preferences not caught by clang-format -These are flexible. You can ignore them if it looks or works better to -for one reason or another. +- Bug and crash reports. You must follow the instructions in the issue templates and provide the + information requested. -Use `auto` if the type of a variable can be deduced automatically, instead of -redeclaring the returned value's type. Additionally, auto should be used when a -constructor takes arguments. +- Feature requests can be made via Issues. Please check to ensure nobody else has requested the same feature. -```cpp -auto x = ; // ok -auto x = QString::number(3); // ok -QString x; // ok -QString x = "foo"; // ok -auto x = QString("foo"); // ok +- Do not make insubstantial or pointless changes. -auto x = QString(); // avoid -QString x(); // avoid -QString x("foo"); // avoid -``` +- Changes to project rules / policy / governance will not be entertained, except from significant + long-term contributors. These changes should not be addressed through contribution channels. -Put newlines around logical units of code, and after closing braces. If the -most reasonable logical unit of code takes only a single line, it should be -merged into the next single line logical unit if applicable. -```cpp -// multiple units -auto x = ; // unit 1 -auto y = ; // unit 2 +## Merge timelines -auto x = ; // unit 1 -emit this->y(); // unit 2 - -auto x1 = ; // unit 1 -auto x2 = ; // unit 1 -auto x3 = ; // unit 1 - -auto y1 = ; // unit 2 -auto y2 = ; // unit 2 -auto y3 = ; // unit 2 - -// one unit -auto x = ; -if (x...) { - // ... -} - -// if more than one variable needs to be used then add a newline -auto x = ; -auto y = ; - -if (x && y) { - // ... -} -``` - -Class formatting: -```cpp -//! Doc comment summary -/// Doc comment body -class Foo: public QObject { - // The Q_OBJECT macro comes first. Macros are ; terminated. - Q_OBJECT; - QML_ELEMENT; - QML_CLASSINFO(...); - // Properties must stay on a single line or the doc generator won't be able to pick them up - Q_PROPERTY(...); - /// Doc comment - Q_PROPERTY(...); - /// Doc comment - Q_PROPERTY(...); - -public: - // Classes should have explicit constructors if they aren't intended to - // implicitly cast. The constructor can be inline in the header if it has no body. - explicit Foo(QObject* parent = nullptr): QObject(parent) {} - - // Instance functions if applicable. - static Foo* instance(); - - // Member functions unrelated to properties come next - void function(); - void function(); - void function(); - - // Then Q_INVOKABLEs - Q_INVOKABLE function(); - /// Doc comment - Q_INVOKABLE function(); - /// Doc comment - Q_INVOKABLE function(); - - // Then property related functions, in the order (bindable, getter, setter). - // Related functions may be included here as well. Function bodies may be inline - // if they are a single expression. There should be a newline between each - // property's methods. - [[nodiscard]] QBindable bindableFoo() { return &this->bFoo; } - [[nodiscard]] T foo() const { return this->foo; } - void setFoo(); - - [[nodiscard]] T bar() const { return this->foo; } - void setBar(); - -signals: - // Signals that are not property change related go first. - // Property change signals go in property definition order. - void asd(); - void asd2(); - void fooChanged(); - void barChanged(); - -public slots: - // generally Q_INVOKABLEs are preferred to public slots. - void slot(); - -private slots: - // ... - -private: - // statics, then functions, then fields - static const foo BAR; - static void foo(); - - void foo(); - void bar(); - - // property related members are prefixed with `m`. - QString mFoo; - QString bar; - - // Bindables go last and should be prefixed with `b`. - Q_OBJECT_BINDABLE_PROPERTY(Foo, QString, bFoo, &Foo::fooChanged); -}; -``` - -### Linter -All contributions should pass the linter. - -Note that running the linter requires disabling precompiled -headers and including the test codepaths: -```sh -$ just configure debug -DNO_PCH=ON -DBUILD_TESTING=ON -$ just lint-changed -``` - -If the linter is complaining about something that you think it should not, -please disable the lint in your MR and explain your reasoning if it isn't obvious. - -### Tests -If you feel like the feature you are working on is very complex or likely to break, -please write some tests. We will ask you to directly if you send in an MR for an -overly complex or breakable feature. - -At least all tests that passed before your changes should still be passing -by the time your contribution is ready. - -You can run the tests using `just test` but you must enable them first -using `-DBUILD_TESTING=ON`. - -### Documentation -Most of quickshell's documentation is automatically generated from the source code. -You should annotate `Q_PROPERTY`s and `Q_INVOKABLE`s with doc comments. Note that the parser -cannot handle random line breaks and will usually require you to disable clang-format if the -lines are too long. - -Before submitting an MR, if adding new features please make sure the documentation is generated -reasonably using the `quickshell-docs` repo. We recommend checking it out at `/docs` in this repo. - -Doc comments take the form `///` or `///!` (summary) and work with markdown. -You can reference other types using the `@@[Module.][Type.][member]` shorthand -where all parts are optional. If module or type are not specified they will -be inferred as the current module. Member can be a `property`, `function()` or `signal(s)`. -Look at existing code for how it works. - -Quickshell modules additionally have a `module.md` file which contains a summary, description, -and list of headers to scan for documentation. - -## Contributing - -### Commits -Please structure your commit messages as `scope[!]: commit` where -the scope is something like `core` or `service/mpris`. (pick what has been -used historically or what makes sense if new). Add `!` for changes that break -existing APIs or functionality. - -Commit descriptions should contain a summary of the changes if they are not -sufficiently addressed in the commit message. - -Please squash/rebase additions or edits to previous changes and follow the -commit style to keep the history easily searchable at a glance. -Depending on the change, it is often reasonable to squash it into just -a single commit. (If you do not follow this we will squash your changes -for you.) - -### Sending patches -You may contribute by submitting a pull request on github, asking for -an account on our git server, or emailing patches / git bundles -directly to `outfoxxed@outfoxxed.me`. - -### Getting help -If you're getting stuck, you can come talk to us in the -[quickshell-development matrix room](https://matrix.to/#/#quickshell-development:outfoxxed.me) -for help on implementation, conventions, etc. -Feel free to ask for advice early in your implementation if you are -unsure. +We handle work for the most part on a push basis. If your PR has been ignored for a while +and is still relevant please bump it. diff --git a/HACKING.md b/HACKING.md new file mode 100644 index 0000000..69357f1 --- /dev/null +++ b/HACKING.md @@ -0,0 +1,226 @@ +## Development + +Install the dependencies listed in [BUILD.md](BUILD.md). +You probably want all of them even if you don't use all of them +to ensure tests work correctly and avoid passing a bunch of configure +flags when you need to wipe the build directory. + +The dependencies are also available as a nix shell or nix flake which we recommend +using with nix-direnv. + +Quickshell uses `just` for common development command aliases. + +Common aliases: +- `just configure [ [extra cmake args]]` (note that you must specify debug/release to specify extra args) +- `just build` - runs the build, configuring if not configured already. +- `just run [args]` - runs quickshell with the given arguments +- `just clean` - clean up build artifacts. `just clean build` is somewhat common. + +### Formatting +All contributions should be formatted similarly to what already exists. +Group related functionality together. + +Run the formatter using `just fmt`. +If the results look stupid, fix the clang-format file if possible, +or disable clang-format in the affected area +using `// clang-format off` and `// clang-format on`. + +#### Style preferences not caught by clang-format +These are flexible. You can ignore them if it looks or works better to +for one reason or another. + +Use `auto` if the type of a variable can be deduced automatically, instead of +redeclaring the returned value's type. Additionally, auto should be used when a +constructor takes arguments. + +```cpp +auto x = ; // ok +auto x = QString::number(3); // ok +QString x; // ok +QString x = "foo"; // ok +auto x = QString("foo"); // ok + +auto x = QString(); // avoid +QString x(); // avoid +QString x("foo"); // avoid +``` + +Put newlines around logical units of code, and after closing braces. If the +most reasonable logical unit of code takes only a single line, it should be +merged into the next single line logical unit if applicable. +```cpp +// multiple units +auto x = ; // unit 1 +auto y = ; // unit 2 + +auto x = ; // unit 1 +emit this->y(); // unit 2 + +auto x1 = ; // unit 1 +auto x2 = ; // unit 1 +auto x3 = ; // unit 1 + +auto y1 = ; // unit 2 +auto y2 = ; // unit 2 +auto y3 = ; // unit 2 + +// one unit +auto x = ; +if (x...) { + // ... +} + +// if more than one variable needs to be used then add a newline +auto x = ; +auto y = ; + +if (x && y) { + // ... +} +``` + +Class formatting: +```cpp +//! Doc comment summary +/// Doc comment body +class Foo: public QObject { + // The Q_OBJECT macro comes first. Macros are ; terminated. + Q_OBJECT; + QML_ELEMENT; + QML_CLASSINFO(...); + // Properties must stay on a single line or the doc generator won't be able to pick them up + Q_PROPERTY(...); + /// Doc comment + Q_PROPERTY(...); + /// Doc comment + Q_PROPERTY(...); + +public: + // Classes should have explicit constructors if they aren't intended to + // implicitly cast. The constructor can be inline in the header if it has no body. + explicit Foo(QObject* parent = nullptr): QObject(parent) {} + + // Instance functions if applicable. + static Foo* instance(); + + // Member functions unrelated to properties come next + void function(); + void function(); + void function(); + + // Then Q_INVOKABLEs + Q_INVOKABLE function(); + /// Doc comment + Q_INVOKABLE function(); + /// Doc comment + Q_INVOKABLE function(); + + // Then property related functions, in the order (bindable, getter, setter). + // Related functions may be included here as well. Function bodies may be inline + // if they are a single expression. There should be a newline between each + // property's methods. + [[nodiscard]] QBindable bindableFoo() { return &this->bFoo; } + [[nodiscard]] T foo() const { return this->foo; } + void setFoo(); + + [[nodiscard]] T bar() const { return this->foo; } + void setBar(); + +signals: + // Signals that are not property change related go first. + // Property change signals go in property definition order. + void asd(); + void asd2(); + void fooChanged(); + void barChanged(); + +public slots: + // generally Q_INVOKABLEs are preferred to public slots. + void slot(); + +private slots: + // ... + +private: + // statics, then functions, then fields + static const foo BAR; + static void foo(); + + void foo(); + void bar(); + + // property related members are prefixed with `m`. + QString mFoo; + QString bar; + + // Bindables go last and should be prefixed with `b`. + Q_OBJECT_BINDABLE_PROPERTY(Foo, QString, bFoo, &Foo::fooChanged); +}; +``` + +Use lowercase .h suffixed Qt headers, e.g. `` over ``. + +### Linter +All contributions should pass the linter. + +Note that running the linter requires disabling precompiled +headers and including the test codepaths: +```sh +$ just configure debug -DNO_PCH=ON -DBUILD_TESTING=ON +$ just lint-changed +``` + +If the linter is complaining about something that you think it should not, +please disable the lint in your MR and explain your reasoning if it isn't obvious. + +### Tests +If you feel like the feature you are working on is very complex or likely to break, +please write some tests. We will ask you to directly if you send in an MR for an +overly complex or breakable feature. + +At least all tests that passed before your changes should still be passing +by the time your contribution is ready. + +You can run the tests using `just test` but you must enable them first +using `-DBUILD_TESTING=ON`. + +### Documentation +Most of quickshell's documentation is automatically generated from the source code. +You should annotate `Q_PROPERTY`s and `Q_INVOKABLE`s with doc comments. Note that the parser +cannot handle random line breaks and will usually require you to disable clang-format if the +lines are too long. + +Make sure new files containing doc comments are added to a `module.md` file. +See existing module files for reference. + +Doc comments take the form `///` or `///!` (summary) and work with markdown. +You can reference other types using the `@@[Module.][Type.][member]` shorthand +where all parts are optional. If module or type are not specified they will +be inferred as the current module. Member can be a `property`, `function()` or `signal(s)`. +Look at existing code for how it works. + +If you have made a user visible change since the last tagged release, describe it in +[changelog/next.md](changelog/next.md). + +## Contributing + +### Commits +Please structure your commit messages as `scope: commit` where +the scope is something like `core` or `service/mpris`. (pick what has been +used historically or what makes sense if new). + +Commit descriptions should contain a summary of the changes if they are not +sufficiently addressed in the commit message. + +Please squash/rebase additions or edits to previous changes and follow the +commit style to keep the history easily searchable at a glance. +Depending on the change, it is often reasonable to squash it into just +a single commit. (If you do not follow this we will squash your changes +for you.) + +### Getting help +If you're getting stuck, you can come talk to us in the +[quickshell-development matrix room](https://matrix.to/#/#quickshell-development:outfoxxed.me) +for help on implementation, conventions, etc. There is also a bridged [discord server](https://discord.gg/UtZeT3xNyT). +Feel free to ask for advice early in your implementation if you are +unsure. diff --git a/README.md b/README.md index 4491d24..365bdb5 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,9 @@ This repo is hosted at: - https://github.com/quickshell-mirror/quickshell # Contributing / Development -See [CONTRIBUTING.md](CONTRIBUTING.md) for details. +- [HACKING.md](HACKING.md) - Development instructions and policy. +- [CONTRIBUTING.md](CONTRIBUTING.md) - Contribution policy. +- [BUILD.md](BUILD.md) - Packaging and build instructions. #### License