From d09afc21bcfb1dadaf51c5c13e84892463eb9524 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 22 Jul 2026 12:43:36 +0200 Subject: [PATCH] chore: single source of truth for contribution guidelines Signed-off-by: Jakob Borg --- dev/contributing.rst | 195 ------------------------------------------- dev/index.rst | 1 - dev/intro.rst | 192 ++---------------------------------------- 3 files changed, 5 insertions(+), 383 deletions(-) delete mode 100644 dev/contributing.rst diff --git a/dev/contributing.rst b/dev/contributing.rst deleted file mode 100644 index 7de1738bd..000000000 --- a/dev/contributing.rst +++ /dev/null @@ -1,195 +0,0 @@ -.. _contribution-guidelines: - -Contribution Guidelines -======================= - -Getting Started ---------------- - -So you want to contribute? Great! Here's a short checklist with the most -important points: - -- Don't worry. You are not expected to get everything right on the first - attempt, we'll guide you through it. - -- Make sure there is an `issue - `__ that describes the - change you want to do. If the thing you want to do does not have an issue - yet, please file one before starting work on it. - -- Fork the repository and make your changes in a new branch. If you already - have push access to the Syncthing repository, do *not* create a new branch - there. We do all changes as pull requests from personal forks. - -Authorship ----------- - -All code authors are listed in the AUTHORS file. When your first pull request -is accepted, the maintainer will add your details to the AUTHORS file, the -NICKS file and the list of authors in the GUI. Commits must be made with the -same name and email as listed in the AUTHORS file. To accomplish this, ensure -that your git configuration is set correctly prior to making your first -commit:: - - $ git config --global user.name "Jane Doe" - $ git config --global user.email janedoe@example.com - -You must be reachable on the given email address. If you do not wish to use -your real name for whatever reason, using a nickname or pseudonym is perfectly -acceptable. - -Team Membership ---------------- - -Contributing useful changes via pull requests will at some point get you -invited to the ``contributors`` team -- typically, after having contributed -five or more nontrivial changes during the last year. This team gives you -push access to most repositories, subject to the guidelines below. - -The ``maintainers`` team is for long standing contributors with the added -responsibility of reviewing major changes. - -Code Review ------------ - -Commits will generally fall into one of the three categories below, with -different requirements on review. - -Trivial: - A small change or refactor that is obviously correct. These may be pushed - without review by any member of the ``maintainers`` team. Examples: - `removing dead code `__, - :commit:`updating values <2aa028facb7ccbe48e85c8c444501cc3fb38ef24>`. - -Minor: - A new feature, bugfix or refactoring that may need extra eyes on it to weed - out mistakes, but is architecturally simple or at least uncontroversial. - Minor changes must go through a pull request and can be merged on approval - by any other developer on the ``contributors`` or ``maintainers`` team. - Examples: `adding caching `__, - `fixing a small bug `__. - -Major: - A complex new feature or bugfix, a large refactoring, or a change to the - underlying architecture of things. A major change must be reviewed by a - member of the ``maintainers`` team. - -Infrastructure: - Changes to the build system, release process, or other infrastructure - components. Iteration may sometimes happen on branches in the main repo, - to test interactions with GitHub Actions, etc. These should be reviewed by - a member of the ``maintainers`` team. - -Coding Style ------------- - -General -~~~~~~~ - -- All text files use Unix line endings. The git settings already present in - the repository attempt to enforce this. - -- When making changes, follow the brace and parenthesis style of the - surrounding code. - -Go Specific -~~~~~~~~~~~ - -- Follow the conventions laid out in `Effective - Go `__ as much as makes - sense. The review guidelines in `Go Code Review Comments - `__ should generally - be followed. - -- Each commit should be ``go fmt`` clean. - -- Imports are grouped per ``goimports`` standard; that is, standard - library first, then third party libraries after a blank line. - -Commits -------- - -- Commit messages (and pull request titles) should follow the [conventional - commits](https://www.conventionalcommits.org/en/v1.0.0/) specification and - be in lower case. - -- We use a scope description in the commit message subject. This is the - component of Syncthing that the commit affects. For example, ``gui``, - ``protocol``, ``scanner``, ``upnp``, etc -- typically, the part after - ``lib/`` or ``cmd/`` in the package path. If the commit doesn't affect a - specific component, such as for changes to the build system or - documentation, the scope should be omitted. The same goes for changes - that affect many components which would be cumbersome to list. - -- Commits that resolve an existing issue must include the issue number - as ``(fixes #123)`` at the end of the commit message subject. A correctly - formatted commit message looks like this:: - - feat(dialer): add env var to disable proxy fallback (fixes #3006) - -- If the commit message subject doesn't say it all, one or more paragraphs of - describing text should be added to the commit message. This should explain - why the change is made and what it accomplishes. - -- When drafting a pull request, please feel free to add commits with - corrections and merge from ``main`` when necessary. This provides a clear time - line with changes and simplifies review. Do not, in general, rebase your - commits, as this makes review harder. - -- Pull requests are merged to ``main`` using squash merge. The "stream of - consciousness" set of commits described in the previous point will be reduced - to a single commit at merge time. The pull request title and description will - be used as the commit message. - -Tests ------ - -Yes please, do add tests when adding features or fixing bugs. Also, when a -pull request is filed a number of automatic tests are run on the code. This -includes: - -- That the code actually builds and the test suite passes. - -- That the code is correctly formatted (``go fmt``). - -- That the commits are based on a reasonably recent ``main``. - -- That the author is listed in AUTHORS. - -- That the output from ``go lint`` and ``go vet`` is clean. (This checks for a - number of potential problems the compiler doesn't catch.) - -Branches --------- - -- ``main`` is the main branch containing good code that will end up - in the next release. You should base your work on it. It won't ever - be rebased or force-pushed to. - -- ``vx.y`` branches exist to make patch releases on otherwise obsolete - minor releases. Should only contain fixes cherry picked from ``main``. - Don't base any work on them. - -- ``infrastructure`` is a specific branch from which builds for the - infrastructure components (usage reporting server, crash reporting server, - relay pool server, etc) are sometimes made. It may be ahead of ``main``. - Do not base any work on it. - -- Other branches are probably topic branches and may be subject to rebasing. - Don't base any work on them unless you specifically know otherwise. - Generally, avoid creating branches on the main repo, preferring instead to - have topic branches on your own fork. - -Tags ----- - -All releases are tagged semver style as ``vx.y.z``. The maintainer doing the -release signs the tag using their GPG key. - -Licensing ---------- - -All contributions are made under the same MPLv2 license as the rest of the -project, except documentation, user interface text and translation strings -which are licensed under the Creative Commons Attribution 4.0 International -License. You retain the copyright to code you have written. diff --git a/dev/index.rst b/dev/index.rst index 919c56848..bce9cc482 100644 --- a/dev/index.rst +++ b/dev/index.rst @@ -8,7 +8,6 @@ Development Introduction web building - contributing debugging crashrep device-ids diff --git a/dev/intro.rst b/dev/intro.rst index 6c52ae657..9df44ccd7 100644 --- a/dev/intro.rst +++ b/dev/intro.rst @@ -20,191 +20,9 @@ if you would like to add functionality to Syncthing itself, or correct a bug or two in there, please read on. -Contributing to the Syncthing Core ----------------------------------- +Contributing to Syncthing +------------------------- -First of all, follow :ref:`building` to get your workspace set up correctly. -Syncthing is written mainly in `Go `__ which has some -fairly specific opinions on the required directory layout. If you're new to -Go, don't fear -- it's a small language and easy to learn. There's a `wealth -of resources `__ on -the web to help you get up to speed, and many people joining the project have -done so with it being their first contact with Go. - -When you are ready to start hacking, take a quick glance at the :ref:`contribution-guidelines` -to know what to expect and to make the process smoother. The main take away is -to keep the code clean, base it on the ``main`` branch, and we'll sort out -the rest once you file a pull request. - - -Source Code Layout -~~~~~~~~~~~~~~~~~~ - -In the source repository you'll find a tree of various packages and -directories. There is some Go code at the top level, but it's basically scripts -for the build system. The actual code lives in the ``cmd/syncthing`` and -``lib`` directories. The web GUI lives in ``gui``. The rest is as follows. - -assets/ - Various graphical assets -- the logo. - -bin/ - Compiled packages, generated by the build process. - -cmd/ - Commands either built as end products or used by the build process itself. - - stdiscosrv/ - Global discovery server for the Syncthing project. - - strelaysrv/ - Relay server for the Syncthing project. - - syncthing/ - Synchronizes files between devices... - - dev/ - Development related utilities. - - stcompdirs/ - Compares two directories (debugging utility). - - stevents/ - Displays event trace from a remote ``syncthing`` using the API (debugging utility). - - stfileinfo/ - Shows information about a file, in the same manner ``syncthing`` would see it (debugging utility). - - stfinddevice/ - Looks up a device on a global discovery server (debugging utility). - - infra/ - Infrastructure related utilities. - -etc/ - Startup scripts and integration files. Included as-is in the release packages. - -gui/ - The web GUI source. Gets compiled into the ``syncthing`` binary by way of ``genassets`` and the build process. - -lib/ - Contains all packages that make up the parts of ``syncthing``. - - beacon/ - Multicast and broadcast UDP beacons. Used by the local discovery system. - - config/ - Parses, validates and saves configuration files. - - db/ - Stores and processes file index information in a database on disk. - - discover/ - The local and global device discovery -- maps device IDs to IP and port tuples. - - events/ - The event subsystem, handles emitting of and subscribing to events across the other packages. - - ignore/ - Parses the ``.stignore`` file and matches it against file paths. - - model/ - Ties together many parts of ``syncthing`` and handles the main logic of synchronizing files with other devices. - - osutil/ - Abstracts away certain OS specific quirks. - - protocol/ - Implementation of the BEP protocol. - - rc/ - Remote controls a Syncthing process over the REST API. - - scanner/ - Looks for changes to files and hashes them as appropriate. - - stats/ - Records statistics about devices and folders. - - sync/ - Provides optional debugging on top of the regular Mutex / RWMutex primitives. - - upgrade/ - Downloads and performs upgrade of the running binary. - - upnp/ - Discovers UPnP devices and sets up port mappings for incoming connections. - - versioner/ - Provides file versioning algorithms; simple, staggered and external. - -man/ - Manual pages, generated from the documentation. - -script/ - Various utility scripts for auto generating stuff and so on. - - genassets.go - Generates asset files that are compiled into ``syncthing`` as part of the build process (build utility). - - translate.go - Generates translation source based on the HTML source (build utility). - -test/ - The integration test suite. - -vendor/ - Locally vendored copies of external dependencies. - - -Why are you being so hard on my pull request? -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -A pull request looks a little different depending on whether you're on the -"contributor" or "maintainer" side. The contributor says: - - I implemented a new feature in your project for you! - -However, the maintainer hears: - - I wrote some code. I'd like you to test, support, document and - maintain it for me forever. - -The maintainer will want to make sure that the code is something we feel -comfortable taking that responsibility for. That means well tested, clear -implementation, fits into the overall architecture, etc. - -But perhaps the existing code doesn't fulfill this to start with; is it then -fair to expect it from a change in a pull request? For example asking for a -test or documentation, where there is none before. Well, the existing code has -some advantage just by being legacy; - -- Perhaps there isn't a test, but we know this code works because it's - been running in production for a long time without complaints. Then - it's fair to expect tests from code replacing it. - -- Perhaps there isn't a test, and your code fixes a bug with the code. - That just highlights that there *should have been* a test to start - with, and this is the optimal time to add one. - -- Perhaps how the code works (or what exactly it does) isn't clear to the - reviewer. A test will clarify and lock this down, and also prevent us - from *inadvertently breaking it later*. - -Another thing that the maintainer might be hard about is whether the -code actually solves the *entire* problem, or at least enough of it to -stand on its own. This will be more relevant to new features than -bugfixes and includes questions like; - -- Is the feature general enough to be used by other users? If not, do - we really need it or can it be implemented as part of something more - general? - -- Is the feature completely implemented? That is, if a new feature is - added it should be available in the GUI, emit relevant trace - information to enable debugging, be correctly saved in the - configuration, etc. If components of this are missing, that's work - the maintainer will have to do after accepting the pull request. - -All in all, a great pull request creates less work for the maintainer, -not more. +Please see `the contribution guidelines +`__ in the +main repository. \ No newline at end of file