Changes

From Gramps
Synced from repo@41d611f via publish.py
{{man index|6.0}}

== Overview ==

Normative reference for addon authors. Conceptual / how-to material lives in the other section pages; this page enumerates the guidelines and is the one to cite in code review.

== Conventions ==

RFC 2119 keywords, with our short forms:

{|
! Keyword
! Meaning
|-
| '''MUST''' / '''MUST NOT'''
| Required; a violation is a defect
|-
| '''SHOULD''' / '''SHOULD NOT'''
| Strongly recommended; deviate only with a stated reason
|-
| '''MAY'''
| Allowed
|}

Where a rule has a known origin — an upstream PR, a maintainer ruling, a Mantis bug — it's cited inline so the rule is auditable.

== Structure ==

* '''MUST''': the addon's folder name matches the <code>id</code> in <code>.gpr.py</code>.
* '''MUST''': <code>.gpr.py</code> declares <code>gramps_target_version</code> matching the Gramps minor the addon targets.
* '''MUST''': <code>fname</code> points to an implementation module shipped in the same folder.
* '''MUST''' (Gramps 6.0): the addon is physically present under the plugin path. 6.0 plugin discovery does not follow symlinks. '''MAY''' (Gramps 6.1+): the addon folder is reached via a symlink. 6.1 plugin discovery follows symlinks, with realpath-based dedup so symlink loops terminate. The symlink test is skipped on Windows because the platform's symlink behavior is inconsistent without elevated privileges; on Windows a physical copy remains the recommended approach even on 6.1+.
* '''MUST NOT''': import <code>register</code>, <code>GRAMPLET</code>, <code>STABLE</code>, <code>_</code>, or any other name Gramps injects into the <code>.gpr.py</code> namespace.
* '''MUST NOT''': add <code>__init__.py</code> to the addon directory itself. The plugin loader puts the addon dir on <code>sys.path</code> and imports <code>&lt;Addon&gt;.py</code> by name; making the addon dir a regular package disturbs that resolution and can trigger the [https://gramps-project.org/bugs/view.php?id=12691 Mantis 12691] submodule-binding trap. (See [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development_-_Testing#why-tests__init__py-exists|08-testing → Why `tests/__init__.py` exists]].)
* '''MUST''' (<code>TOOL</code> kind): register an <code>optionclass</code> even when the tool takes no options. Gramps refuses to load a <code>TOOL</code> without one; an empty <code>tool.ToolOptions</code> subclass is sufficient.
* '''SHOULD''': ship a <code>po/</code> directory with at least <code>template.pot</code> if any user-visible string exists.
* '''SHOULD''': ship a <code>tests/</code> package with an <code>__init__.py</code> marker and at least one test. The marker keeps dotted-path loading deterministic and is the prerequisite for shared test setup.
* '''MAY''': ship multiple plugin kinds from a single addon (multiple <code>register(...)</code> calls in one <code>.gpr.py</code>).

== Source location ==

* '''MUST''': edit addon source in <code>addons-source/</code>, never in the live plugin directory. The auto-sync runs source → installed plugin one-way; edits in the live dir are silently overwritten on the next source save.

== Translation ==

The full how-to (registration setup, <code>make.py</code> lifecycle, Glade runtime-override pattern, function reference) lives in [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development_-_Internationalization|12-internationalization]]. The rules below are what code review enforces.

* '''MUST''': wrap every user-visible string with <code>_()</code>.
* '''MUST NOT''': <code>import _</code> in <code>.gpr.py</code> — Gramps' plugin loader injects it. Implementation modules '''MUST''' bind it explicitly via <code>_ = glocale.get_addon_translator(__file__).gettext</code>.
* '''MUST NOT''': wrap an f-string or <code>.format()</code> result in a translation function. <code>xgettext</code> cannot extract dynamically built strings.
** '''Bad:''' <code>_(f&quot;User {name}&quot;)</code>, <code>_(&quot;User {}&quot;.format(name))</code>
** '''Good:''' <code>_(&quot;User %s&quot;) % name</code>
* '''MUST''' (Glade): translatable strings in <code>.glade</code> / <code>.ui</code> files are '''not''' picked up by the addon translation tooling — the extractor only sees Python. For each translatable Glade string, give the widget a meaningful <code>id</code>, mark the string with <code>translatable=&quot;yes&quot;</code> (optionally with a <code>&quot;context|&quot;</code> prefix), and override the label at runtime in Python: <code>self.get_widget(&quot;place_name_label&quot;).set_label(_(&quot;place|Name:&quot;))</code>.
* '''SHOULD''': use <code>ngettext(singular, plural, n)</code> for plural forms.
* '''SHOULD''': use the pipe-prefix form <code>_(&quot;Context|String&quot;)</code> whenever a word could carry multiple senses (e.g. <code>_(&quot;book|Title&quot;)</code> vs <code>_(&quot;person|Title&quot;)</code>). This is the convention used throughout <code>addons-source</code> and is what translators see in the <code>.po</code> file. The two-arg form <code>_(msg, context)</code> works equivalently. '''MUST NOT''' call <code>pgettext</code> or <code>sgettext</code> directly — go through <code>_</code>.
* '''SHOULD''': use <code>N_(&quot;…&quot;)</code> to mark a string for extraction without translating it at call time (e.g. for module-level constants that are translated later when displayed).

== Runtime ==

<ul>
<li>'''MUST''': perform every database write inside a <code>DbTxn</code>:
<syntaxhighlight lang="python">with DbTxn(_("Adding example"), db) as trans:
db.add_person(person, trans)</syntaxhighlight></li>
<li>'''MUST''': declare runtime imports in <code>requires_mod</code> using the ''importable'' module name (<code>PIL</code>), not the PyPI distribution name (<code>Pillow</code>).</li>
<li>'''MUST''': verify each <code>requires_mod</code> entry with <code>importlib.util.find_spec(&quot;&lt;name&gt;&quot;)</code> on a system with the package installed before publishing.</li>
<li>'''MUST''': use <code>requires_gi</code> for GObject-Introspection bindings, with version strings. The version pin '''must match what the code actually imports''' at runtime — pins can drift between Gramps minors (e.g. GExiv2 handling was rewritten on <code>maintenance/gramps61</code> per addons-source PR 829), so verify the pin against the target branch's related code, not just the previous branch's working declaration.</li>
<li>'''SHOULD''': use handles (<code>PersonHandle</code>, etc.) for internal traversal; reserve Gramps IDs (<code>I0001</code>, …) for user-facing display. Handles are internal and stable; Gramps IDs are user-editable and rewritten in bulk by the Reorder Gramps IDs tool.</li>
<li>'''SHOULD''': import only from <code>gramps.gen.*</code>. <code>gramps.gui.*</code> and <code>gramps.plugins.*</code> are internal to the shipped distribution and break across Gramps versions.</li>
<li>'''SHOULD''': use a module-level logger (<code>LOG = logging.getLogger(__name__)</code>); '''MUST NOT''' use <code>print()</code> for diagnostic output.</li>
<li>'''SHOULD''': raise existing exceptions from <code>gramps.gen.errors</code> and <code>gramps.gen.db.exceptions</code> before inventing a new class.</li>
<li>'''SHOULD''': raise <code>HandleError</code> for invalid or missing handles.</li>
<li>'''SHOULD''': compare backlink class names by string. <code>db.find_backlink_handles(handle)</code> yields <code>(class_name, handle)</code> tuples where <code>class_name</code> is <code>&quot;Person&quot;</code> / <code>&quot;Family&quot;</code> / … as a <code>str</code>, not the Python class — <code>if cls is Person:</code> always evaluates <code>False</code>.</li>
<li>'''MAY''': introduce a new exception class only when none of the existing ones accurately represent the error condition.</li></ul>

== Testing ==

<ul>
<li><p>'''MUST''': use stdlib <code>unittest</code> — never <code>pytest</code>. Gramps itself standardises on <code>unittest</code>, which keeps addon tests contributable upstream.</p></li>
<li><p>'''MUST''': name test files <code>test_*.py</code> and place them in a <code>tests/</code> package alongside the addon module.</p></li>
<li><p>'''MUST''': scope platform-specific tests with the correct prefix:</p>
{|
! Prefix
! Where it runs
|-
| <code>test_*.py</code>
| All platforms
|-
| <code>test_linux_*.py</code>
| Linux only
|-
| <code>test_windows_*.py</code>
| Windows only
|-
| <code>test_integration_*.py</code>
| Linux only — full-pipeline / DB-backed
|}
</li>
<li><p>'''MUST''': tests run cleanly without the addon's <code>requires_mod</code> dependencies installed in the Python that runs them — mock at the import boundary, or skip cleanly with <code>@unittest.skipUnless(...)</code>. Mac contributors can't easily install addon deps into the Gramps Python, and there's no Gramps debug-mode on Mac. (Gary Griffin, 2026-05-16.)</p></li>
<li><p>'''SHOULD''': ship a regression test with every bug fix that '''fails pre-fix and passes post-fix'''. Doc-only PRs are the only exception.</p></li>
<li><p>'''SHOULD''': prefer <code>example.gramps</code>-backed tests over mocked DBs for DB-traversal logic — real data has cross-typed backlinks and ID-normalisation shapes that mocks don't reproduce.</p></li>
<li><p>'''MAY''': ship mocked unit tests alongside real-DB tests as complementary coverage.</p></li></ul>

== Coding style ==

The full Python coding standard — Black, Python 3.10+ type hints (<code>X | None</code>, <code>list[X]</code>), Sphinx docstrings, import grouping with comment headers, class-header navigation comments, <code>cb_</code> callback prefix — is specified in <code>../gramps/AGENTS.md</code> and applies to all Gramps-related Python.

* '''MUST''': every new <code>.py</code> file carries a GPL-2.0-or-later license header with copyright.
* '''MUST''': any code in <code>gramps.gen.*</code> does not import from any other Gramps submodule (<code>gen</code> stays self-contained). Addon code SHOULD uphold the same discipline against itself: factor pure logic into modules that don't import <code>gramps.gui.*</code> so it stays unit-testable without a display.
* '''SHOULD''': type-hint all public functions and methods, using Python 3.10+ syntax (<code>X | None</code> over <code>Optional[X]</code>, <code>list[X]</code> over <code>typing.List[X]</code>).
* '''SHOULD''': use handle / ID types from <code>gramps.gen.types</code> (<code>PersonHandle</code>, <code>PersonGrampsID</code>, ...) rather than bare <code>str</code>.
* '''SHOULD''': docstring all functions and methods in Sphinx format.
* '''SHOULD''': prefix callbacks <code>cb_*</code>.
* '''SHOULD''': prepend a class-header navigation comment to each class, including <code>unittest.TestCase</code> subclasses. (PR 2326 round 2.)
* '''SHOULD''': run <code>black --check</code> before pushing to repos that enforce it. Gramps core does (pre-commit + CI); addons-source today does NOT.

== Contributor workflow ==

* '''MUST''': one logical fix per PR. Bundling hides mistakes.
* '''MUST''': target the right branch:
** Addon changes (<code>addons-source</code>) → <code>maintenance/gramps60</code>. The maintainer cherry-picks forward to <code>gramps61</code>. (Gary Griffin on addons-source PR 915, 2026-05-24.)
** Core changes (<code>gramps</code>) → <code>maintenance/gramps61</code>. Fixes and cleanups go on the current production branch and forward-merge to <code>master</code>. (jralls on gramps#2298.)
** Reviewer instruction on a specific PR wins over the default targeting. (e.g. Nick-Hall on gramps#2299.)
* '''MUST''': branch from <code>upstream/&lt;base&gt;</code>, not the fork's tracking copy — fork bases drift (e.g. PRs 2315/2316 carried a stray <code>AGENTS.md</code> from the fork).
* '''MUST NOT''': bump the addon's <code>version</code> field in an addons-source PR. The maintainer manages versions centrally. (Caught on PR 911, bug 12572.)
* '''MUST''': a bug-fix PR includes a regression test, or an explicit &quot;no test because X&quot; rationale plus a manual repro. &quot;Add the test later&quot; is not an option.
* '''MUST''': structure the PR body as '''Root cause / Fix / Verified against / Test''', citing <code>path:lines</code> on the branch the PR targets.
* '''MUST''': reference the Mantis bug — gramps core PRs end the body ''and'' the commit message with <code>Fixes #NNNN</code> (no URL, no padding; PR 2322 is the canonical example). addons-source PRs reference the bug in the body.
* '''MUST NOT''': merge across branches. Rebase rather than merge — PRs with merge commits are rejected upstream.
* '''MUST NOT''': cosmetically update in-flight upstream PRs. Parity, &quot;rebase is clean,&quot; and &quot;branch is behind&quot; are not reasons to force-push. Push only when a specific correctness issue needs fixing.
* '''SHOULD''': before writing any fix, check upstream isn't ahead — merged history on the target branch AND <code>master</code>, ''plus'' closed and rejected PRs on the ''affected file'' (not just the bug number). Closed PRs are signal: a closed-unmerged PR with the same fix shape is the maintainer's &quot;no.&quot;
* '''SHOULD''': if a PR already exists for the bug, verify it instead of duplicating. Merged → confirm-and-close; open → review and defer to the maintainer; closed → treat as the maintainer's &quot;no.&quot;
* '''SHOULD''': reproduce against <code>example.gramps</code> first — it's the canonical fixture and &quot;couldn't reproduce&quot; is the most common reason a fix stalls in triage.
* '''MAY''': open as a draft PR for early review or to publish work-in-progress; mark ready when the change is complete and the author has re-read the diff with fresh eyes.

== Verification before commit ==

* '''MUST''': find a test procedure before committing — local run, dry-run, snippet check. Never commit untested changes.
* '''MUST''': treat a green mechanical check (lint, <code>git cherry-pick</code> applies, build green, <code>py_compile</code> exits 0) as evidence of ''that narrow check'', not of correctness. Name what the check verified and what it left unverified.
* '''MUST''': after pushing a PR branch, watch the PR's CI checks until they finish (e.g. <code>gh pr checks &lt;PR#&gt; --watch</code>). Local pre-commit catches static checks only; test failures surface in CI's actual unit-test run.

== Commit messages ==

Commit messages are parsed by scripts that update Mantis BT and generate the ChangeLog / News files for releases. Formatting must be followed precisely.

* '''MUST''': the first line is a short summary, '''≤ 70 characters'''.
* '''MUST''': the description is separated from the summary by a single blank line, and wrapped at '''80 characters'''.
* '''MUST''': describe the change from the user's perspective. Don't recap the diff — <code>git diff</code> exists.
* '''SHOULD''': use complete sentences in the description.
* '''MUST''': reference another commit by its '''full hash''', not a short hash. GitHub auto-hyperlinks full hashes; short hashes in brackets do not link.
* '''MUST''': the Mantis trailer is on the '''last''' line of the commit message, separated from the description by a single blank line.

=== Mantis trailer keywords ===

To '''resolve''' a bug (closes it on commit):

<pre>Fixes #12345
Fixed #12345
Resolves #12345
Resolved #12345
Fixes #12345, #67890</pre>
To '''link''' to a bug (cross-reference without closing):

<pre>Bug #12345
Issue #12345
Report #12345
Bugs #12345, #67890</pre>
Bare numbers (no <code>#</code>) and URLs both miss the auto-link — use the <code>#NNNN</code> form. Note this is the opposite of the convention ''inside'' MantisBT itself, where <code>#NNNN</code> auto-links to another Mantis issue and bare numbers are preferred; here, inside Git commit messages and GitHub PR bodies, <code>#NNNN</code> is what hooks the MantisBT scripts.

For the trailer to wire up on Mantis, the Git '''author''' or '''committer''' has to be a developer on the Mantis bug tracker. The Git name must match the Mantis username or real name, or the Git email must match the Mantis email.

=== gramps core: trailer at the END ===

For gramps core PRs both the '''PR body''' and the '''commit message''' end with <code>Fixes #NNNN</code> (no URL, no padding; PR 2322 is the canonical example). The older URL-at-top form on PR 2317 is the outdated pattern.

=== addons-source: bug reference in PR body ===

addons-source PRs don't use <code>Fixes #NNNN</code> in the commit message; reference the Mantis bug in the PR body instead.

== Adding and removing Python files (gramps core only) ==

When a gramps core PR adds or removes a <code>.py</code> file with translatable strings:

* '''MUST''': update <code>po/POTFILES.in</code> (translatable strings) or <code>po/POTFILES.skip</code> (no translatable strings; deliberately excluded).
* '''MUST''': remove references from both <code>po/POTFILES.in</code> and <code>po/POTFILES.skip</code> when a file is deleted.

Local check:

<syntaxhighlight lang="bash">PYTHONPATH=../../gramps python po/test/po_test.py</syntaxhighlight>
For addons, the per-addon <code>po/template.pot</code> is regenerated by <code>make.py init &lt;Addon&gt;</code> (see [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development|12-packagingK-packaging.md)); there is no equivalent of `POTFILES.in` to maintain by hand.

## See also

- [Overview]]

* [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development_-_Fundamentals|Fundamentals]]
* [[Gramps_6.0_Wiki_Manual_-_Addon_Testing|Testing]]
* [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development_-_Code_Analysis|Code analysis]]
* [[User:Eduralph/Sandbox/Gramps_6.0_Wiki_Manual_-_Addon_Development_-_Packaging|Packaging]]
* <code>../gramps/AGENTS.md</code> — the full Python coding standard inherited here.
* [https://github.com/gramps-project/addons-source/blob/maintenance/gramps60/CONTRIBUTING.md addons-source CONTRIBUTING.md]
* [https://www.gramps-project.org/wiki/index.php/Committing_policies Committing policies] — upstream's commit-message + Mantis-trailer rules.

{{stub}}
28
edits

Navigation menu