Skip to content

Contributing

chrisholloway5 edited this page Sep 8, 2026 · 3 revisions

Contributing

Source of truth: .github/CONTRIBUTING.md, kept in the repository and copied here.

Thanks for your interest in contributing!

This project ships a Code of Conduct — by taking part you agree to abide by it.

Building

See README.md for full build instructions. In short:

  • Visual Studio 2026 (platform toolset v145), 64-bit Windows
  • External libs (OpenSSL 4.0.x, Boost 1.92, PostgreSQL 18 libpq) built under a directory pointed to by the hMailServerLibs environment variable (libraries\build-openssl.ps1, build-boost.ps1 and build-pgsql.ps1 do this)
  • Server solution: hmailserver/source/Server/hMailServer/hMailServer.sln
  • Tools solution: hmailserver/source/Tools/hMailServer Tools.sln
  • Helper scripts live in build/ (build.ps1, build-tests.ps1, run-tests.ps1)

The compiler runs with /WX — code must build warning-free.

Testing

All changes must keep the regression suite green. The suite runs against a live local server instance over SMTP/IMAP/POP3. Setting a machine up for it - the service, SQL Server Compact and the bench database, ClamAV, SpamAssassin, the standard ports and the traps that have broken runs before - is written up in Regression Test Environment; build/preflight-tests.ps1 is the machine-readable version of the same recipe and tells you exactly which step is missing.

Small tasks

If you want to contribute and do not know where to start, the issues labelled good first issue are real, bounded pieces of work, each of which closes a gap recorded in Roadmap.md — none is make-work. Each says what it is for, where to look, how to check it, and what "done" means. Comment on one to claim it; if it turns out to be larger than it looked, say so on the issue rather than growing the change.

Sign-off

Commit with git commit -s. That adds a Signed-off-by: Your Name <you@example.com> trailer, which is your statement that you wrote the change (or have the right to submit it) and that it may be distributed under this project's licence - the Developer Certificate of Origin, the whole text of which is eleven lines. There is no CLA. A pull request whose commits lack the trailer fails the DCO check, which names the commit; the fix is git commit --amend -s and a push.

Pull Requests

  • Branch from master (development branch). Version branches are bug-fix only.
  • Keep changes focused; one logical change per PR.
  • Add or update regression tests for behavior changes.
  • Use parameterised SQL exclusively — never build SQL strings manually.
  • New server-wide optional features should follow the INI-settings pattern: an IniFileSettings getter plus a control in the Server features dialog.
  • Every pull request must pass the required checks: the C# builds with -warnaserror, the hosted C++ server build, CodeQL (C#), dependency review, binary provenance, and the coding-style jobs - editorconfig (.editorconfig: spaces, width 3 for .h/.cpp/.cs), dotnet format --verify-no-changes, and python3 build/add-license-headers.py --check (every source file carries the copyright line and SPDX-License-Identifier: AGPL-3.0-or-later).
  • Control Panel changes are held to five more checks in the same job (three of them new in 6.2.28): every static caption carries an Alt-key mnemonic (build/check-mnemonics.py); a new caption is marked for translation (L("_Save changes") in C#, {loc:L '_Save changes'} in XAML), the English catalogue is regenerated with python3 build/check-localisation.py --write, and all 17 complete languages get a translation - an unmarked or untranslated caption fails CI (build/check-localisation.py, build/check-catalogues.py); every INI setting the server reads has a Control Panel editor (build/check-ini-coverage.py); folder-access decisions stay in ACLManager (build/check-authz-choke-point.py).

The shape of a contribution

flowchart TD
    IDEA["An idea, a bug, or a<br/>good first issue"] --> SIZE{"Does it span more than<br/>about three areas?"}
    SIZE -- yes --> ISSUE["Open an issue first.<br/>A large change is cheaper<br/>to redirect than to rewrite"]
    SIZE -- no --> BRANCH["Branch from master"]
    ISSUE --> BRANCH
    BRANCH --> CODE["Write the change"]
    CODE --> TEST["Add or update a regression test.<br/>For a defect fix, build the<br/>PRE-FIX binary and confirm the<br/>test FAILS against it"]
    TEST --> LOCAL["Build warning-free:<br/>build/build.ps1<br/>build/build-tests.ps1"]
    LOCAL --> PRE["build/preflight-tests.ps1<br/>then build/run-tests.ps1"]
    PRE --> SIGN["git commit -s"]
    SIGN --> PUSH["Push the branch, open the PR"]
    PUSH --> CHECKS{"Nine required checks"}
    CHECKS -- red --> FIXTBL["See the table below -<br/>most have a one-line fix"]
    FIXTBL --> PUSH
    CHECKS -- green --> REVIEW["Maintainer review.<br/>Reasons are recorded on the PR"]
    REVIEW --> MERGE["Rebase merge -<br/>linear history, commits keep<br/>their own messages"]
Loading

When a check goes red

Every one of these fails a required status check, so none of them is optional. The command in the right-hand column is the whole fix.

Check that fails What it means Fix
Signed-off-by on every commit A commit lacks the DCO trailer, or its trailer does not match the commit author's email git commit --amend -s and force-push the branch (this check is not required to merge, but fix it anyway)
editorconfig-checker Indentation drift. .editorconfig sets spaces, width 3, for *.h, *.cpp and *.cs Re-indent; exclusions live in .editorconfig-checker.json at the repository root
dotnet format leaves the C# tree unchanged C# formatting differs from dotnet format's opinion dotnet format on the Tools solution and ControlPanel.csproj
SPDX identifier on every source file — first step A new file has no copyright line or SPDX identifier python3 build/add-license-headers.py with no arguments writes them; --check only reports
… — mnemonics A static Control Panel caption has no Alt-key mnemonic, or two on one view share one python3 build/check-mnemonics.py --list shows what it sees; add _ before the access-key letter
… — localisation A caption is not marked for translation, or the English catalogue is stale Mark it — L("_Save changes") in C#, {loc:L '_Save changes'} in XAML — then python3 build/check-localisation.py --write
… — catalogues A translation dropped a literal name, page title, number or {n} placeholder python3 build/check-catalogues.py names the language and the key
… — INI coverage The server reads a [Settings] key the Control Panel cannot edit Add the editor; python3 build/check-ini-coverage.py names the key
… — authorisation choke point A folder-access decision was made somewhere other than ACLManager Move it into ACLManager
Build Control Panel (.NET 10) / Build .NET tools A warning (CI builds with -warnaserror), a drifted packages.lock.json, or a stale SettingsSearchIndex.g.cs Fix the warning; restore with --locked-mode locally to reproduce
Build server (windows-2025-vs2026) The C++ does not compile on a clean machine — often a header only your tree has build/build.ps1 -Configuration Release from clean
Analyze (csharp) A CodeQL security-and-quality finding The alert names the file and line
Verify committed binaries against the manifest A committed binary changed, went missing, or arrived without an entry Update hmailserver/docs/third-party-binaries.json in the same commit — and if it is the interop wrapper, use build/regenerate-interop.ps1, which rewrites the manifest for you
Dependency review A dependency change brings a high-or-worse advisory Pick a version without it

Note the shape of the localisation rule, because it surprises people: a new caption fails CI in every language until it is translated. All 17 complete languages must carry a translation for every key. That is a deliberate cost — the alternative is a Control Panel that is half-English in eleven of them.

Things that are not style preferences

These are the rules a reviewer will send a pull request back over, each because breaking it has cost something real. Architecture has the full list with the incidents attached.

Rule Why
Parameterised SQL, exclusively Never build SQL by string concatenation. There is no exception, including for DDL — that goes through the macro expanders in SQL/Macros/
Every wait on a pooled thread gets a ceiling A dependency that stops responding otherwise consumes the pool, and the server then accepts mail and never replies
An idle timeout is not a ceiling Idle timeouts re-arm on every byte; a peer dribbling one byte at a time is never idle
Prefer deferral to bouncing A temporary failure costs a retry. A permanent one costs someone their mail
Never make a diagnostic fire on the shipped defaults It puts a Medium entry in every stock install's ERROR log, and fails the fixtures, which assert a clean log
A schema change is four SQL files, not one MS SQL, SQL CE, MySQL and PostgreSQL — plus the DBUpdater row, the probe and REQUIRED_DB_VERSION
New test files must be added to RegressionTests.csproj There is no glob. An unlisted file is invisible, not merely unrun

Architecture

The layering is BO → Persistence → SQL, with Cache in front of the hot reads. All configuration and management goes through the COM API in Server/COM/ — that is the seam the GUI, the test suite and external scripts all use. Networking is Boost.Asio, wrapped by Server/Common/TCPIP/.

Architecture has this drawn out: a component diagram of the whole server, the life of a message from accept to delivery, the six thread pools and what each looks like when it saturates, the RCPT TO decision tree, and a "where to start" table that maps a kind of change to the file it begins in.

License

By contributing you agree that your contributions are licensed under the AGPLv3.

Clone this wiki locally