readme: add architecture documentation and coding conventions (Bug 2020842) by cgsheeh · Pull Request #911 · mozilla-conduit/lando

cgsheeh · 2026-02-09T21:03:40Z

Adds comprehensive project documentation to the README including a detailed architecture overview,
code layout guide, common make commands reference, coding conventions, and commit message
conventions.

Add a `CLAUDE.md` file which outlines some basic details of working in the Lando project, including some code conventions and general layout of the project. We can start with this as a baseline and iterate on it as needed.

github-actions · 2026-02-09T21:03:56Z

View this pull request in Lando to land it once approved.

shtrom

r+ w/ nit re: type hint

shtrom · 2026-02-10T00:32:40Z

CLAUDE.md

+
+## Coding Conventions
+
+- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.


nits:

Suggested change

- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.

- Type-hint function arguments and return values

- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.

We use ruff to enforce ANN rules, so the agent will see test and formatting failures for type hints anyways. I added a note about this, and suggested that the agent should use specific types as much as possible (ie don't use str | int as a type, just pick one of str or int).

zzzeid

I would suggest removing all references to package versions, it doesn't seem necessary and could be a pain to keep up to date.

zzzeid · 2026-02-10T14:15:12Z

CLAUDE.md

Seems like the content of this file could be useful as a general overview of Lando for anyone who wants to learn more about how it works. I don't think we should make it specific for consumption by bots.

We could rename the file to CONTRIBUTING.md, and just add an @CONTRIBUTING.md to CLAUDE.md. Firefox does something in this ilk https://searchfox.org/firefox-main/source/CLAUDE.md.

zzzeid

Can you add more context to the bug as well, as to why we are adding this file? Is there an integration that's going to depend on this?

I would rename this to a more generic explainer and maybe tell that integration to point to that instead.

There's also duplication of info between this and the README so maybe we should just update the README with more comprehensive info.

…d point to readme

cgsheeh · 2026-02-10T15:29:06Z

Can you add more context to the bug as well, as to why we are adding this file? Is there an integration that's going to depend on this?

It's in the file at the very top: "This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository."

Essentially when you run claude from the root of this repo, the first thing it will do it read this file into the model's context. This allows the agent to start working with a basic understanding of the project without having to be repeatedly instructed or steered in the right direction.

Here are the Firefox AGENTS.md/CLAUDE.md files, and Mana has some details on CLAUDE.md as well

I would rename this to a more generic explainer and maybe tell that integration to point to that instead.

There's also duplication of info between this and the README so maybe we should just update the README with more comprehensive info.

Good call. I moved much of the content to the README and made this file reference the README.

cgsheeh · 2026-02-10T15:36:32Z

I would suggest removing all references to package versions, it doesn't seem necessary and could be a pain to keep up to date.

I changed these to instead include a reference to the file where the version can be found.

zzzeid · 2026-02-11T14:59:39Z

Good call. I moved much of the content to the README and made this file reference the README.

Is claude or any other similar tool unable to detect the README itself (i.e., this universally accepted naming convention for a file that should give you the information you need about something)? Or on a per-session or use basis, can the README file be specified for the source of this information? This new file seems redundant/unnecessary.

The README additions themselves are good though but I think the language could be updated to be a little less mechanical.

cgsheeh · 2026-02-11T16:03:47Z

Is claude or any other similar tool unable to detect the README itself (i.e., this universally accepted naming convention for a file that should give you the information you need about something)?

CLAUDE.md is automatically injected into claude's context. Claude could read the README, but it would have to decide to based on a prompt. It may decide to read that file, it may not read it, or it may read it and read many other unnecessary files which pollute the context. Think of this like a repo-level config file for a tool, similar to compose.yaml or .gitignore.

Or on a per-session or use basis, can the README file be specified for the source of this information? This new file seems redundant/unnecessary.
The README additions themselves are good though but I think the language could be updated to be a little less mechanical.

The file is currently only 3 lines, and it's mostly just a "pointer" to the README. The actual substance are the README improvements. Over time, CLAUDE.md may diverge from the readme as it contains more mechanical language and instructions on specific behaviours for the AI to take.

cgsheeh · 2026-03-03T23:21:28Z

I've morphed this PR to include only the README.md changes for the time being.

zzzeid · 2026-03-04T14:11:21Z

README.md

 # Lando

-Lando is an application that applies patches and pushes them to Git and Mercurial repositories.
+Lando is a Django application that applies patches from Phabricator and GitHub pull requests and pushes them to Git and Mercurial repositories. It integrates with Mozilla services (Phabricator, Treestatus, Treeherder) and uses Celery workers for async operations. See `pyproject.toml` for the Django version and other Python dependencies.


nit: the bit about integrating with treestatus is a little misleading (since treestatus is part of Lando, albeit legacy Lando).

I would also maybe add a section titled Project Structure and the bit about pyproject.toml in there, along with the apps breakdown below and maybe combine it with the Code Layout section to make things more readable.

zzzeid · 2026-03-04T14:14:58Z

README.md

+
+### Landing Workers
+
+Separate Docker containers (`hg-landing-worker`, `git-landing-worker`) run Django management commands that loop and poll the database for jobs. Each worker has a corresponding `Worker` model record with configuration details and runs a specific job type.


Might be good to differentiate between remote and local environments here, and perhaps add a bit more generalization about the concept of workers since they're used in various parts of the code at this point other than landing workers.

There's the workers and the compute instance (pod) they run on.

It might be worthwhile mentioning that each worker needs a backing record in the DB in the DB, and that they are then started on the compute instance by specifying the name of the record. Also that there can be multiple workers of a given type, that can be associated to a subset of the Repos for, e.g., permissions granularity.

zzzeid · 2026-03-04T14:17:14Z

README.md

+
+### Authentication
+
+OIDC via Auth0 (`mozilla_django_oidc`) with Django model backend fallback. API authentication via `ApiToken` model.


Suggested change

OIDC via Auth0 (`mozilla_django_oidc`) with Django model backend fallback. API authentication via `ApiToken` model.

Landed uses OIDC for authentication via Auth0 (`mozilla_django_oidc`) and Django's `ModelBackend` as a fallback. There are additional authentication mechanisms via Phabricator API tokens and the `lando.headless_api.models.tokens.ApiToken` model used in other parts of the code.

zzzeid · 2026-03-04T14:21:54Z

README.md

+
+### Frontend
+
+Jinja2 templates (primary) with Django Compressor for asset pipeline. Uses Bulma (version in `package.json`) and FontAwesome (vendored in `src/lando/static_src/legacy/vendor/`) — use APIs from the specific versions pinned in those files.


Suggested change

Jinja2 templates (primary) with Django Compressor for asset pipeline. Uses Bulma (version in `package.json`) and FontAwesome (vendored in `src/lando/static_src/legacy/vendor/`) — use APIs from the specific versions pinned in those files.

Lando uses the Jinja2 templates backend for various user-facing pages. Django Compressor is used for the for asset pipeline both locally (on-demand) and remotely (at deploy time). Lando uses Bulma as a CSS framework and FontAwesome for fonts. For development purposes, check the corresponding Bulma and FontAwesome APIs based on the pinned versions.

NOTE: Bulma version can be found in `package.json`, and FontAwesome is checked into the repo in `src/lando/static_src/legacy/vendor/`.

zzzeid · 2026-03-04T14:22:09Z

README.md

+PostgreSQL (version in `compose.yaml`). Models inherit from `BaseModel` (provides `created_at`/`updated_at`). Migrations live in each app's `migrations/` directory and auto-run on `docker compose up`.
+


Can you reword this to be more human friendly?

zzzeid · 2026-03-04T14:30:59Z

README.md

+- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.
+- Avoid `getattr` and `hasattr`. Referencing attributes directly enables LSP features (go-to-references, rename) and avoids "stringly typed" patterns. Prefer direct attribute access and explicit checks.
+- Add docstrings to functions by default. Even a short one-line docstring is preferred to no docstring.
+- Comments in proper English with full punctuation, for example `# this shouldnt happen` should be `# This shouldn't happen.`.


Suggested change

- Comments in proper English with full punctuation, for example `# this shouldnt happen` should be `# This shouldn't happen.`.

- Added comments should have correct grammar with full punctuation. For example `# this shouldnt happen` should be `# This shouldn't happen.`.

zzzeid · 2026-03-04T14:32:43Z

README.md

+- Avoid `getattr` and `hasattr`. Referencing attributes directly enables LSP features (go-to-references, rename) and avoids "stringly typed" patterns. Prefer direct attribute access and explicit checks.
+- Add docstrings to functions by default. Even a short one-line docstring is preferred to no docstring.
+- Comments in proper English with full punctuation, for example `# this shouldnt happen` should be `# This shouldn't happen.`.
+- No emojis in comments or code. Emojis in user-facing strings are allowed but generally discouraged. Emoji in tests are acceptable where appropriate.


This feels a little out of place.

zzzeid · 2026-03-04T14:34:55Z

README.md

+- Add docstrings to functions by default. Even a short one-line docstring is preferred to no docstring.
+- Comments in proper English with full punctuation, for example `# this shouldnt happen` should be `# This shouldn't happen.`.
+- No emojis in comments or code. Emojis in user-facing strings are allowed but generally discouraged. Emoji in tests are acceptable where appropriate.
+- Linting: ruff (line-length 88, isort, annotations) + black + djlint (jinja profile).


This is also out of place. You can instead maybe say

Suggested change

- Linting: ruff (line-length 88, isort, annotations) + black + djlint (jinja profile).

- Ruff and black, are used to enforce Python code style requirements (see ruff.toml), and djlint is used to enforce Jinja2 code style requirements.

zzzeid · 2026-03-04T14:37:07Z

README.md

+`<module name>: <brief description> (bug <bug number>)`

-### Add requirements
+Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bug numbers are required for code changes, and suggest to file a bug if a developer has not provided a bug number.


Suggested change

Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bug numbers are required for code changes, and suggest to file a bug if a developer has not provided a bug number.

Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bugs are required for code changes. If a bug is not associated with a code change, the author should be asked to file a bug and reference it in the pull request.

s/the author should be asked to file/the author should first file/

zzzeid · 2026-03-04T14:37:39Z

README.md

+Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bug numbers are required for code changes, and suggest to file a bug if a developer has not provided a bug number.

-    make add-requirements
+Commit messages should include a description of the change being made and some context for why the change was made. This added context allows the reviewer to assert the author's expectations and mental models match the actual code changes.


Suggested change

Commit messages should include a description of the change being made and some context for why the change was made. This added context allows the reviewer to assert the author's expectations and mental models match the actual code changes.

Commit messages should include a description of the change being made and some context for why the change was made. This added context allows the reviewer to assert the author's expectations and that the mental models match the actual code changes.

shtrom

LGTM w/ Zeid's suggestions applied.

shtrom · 2026-03-05T05:08:16Z

README.md

+- **headless_api/** - REST API for automation jobs and tokens
+- **try_api/** - Try push API for test builds
+- **treestatus/** - Tree open/closed status monitoring and management
+- **pushlog/** - Mercurial pushlog models


This is mostly a replacement for the Hg pushlog.

Suggested change

- **pushlog/** - Mercurial pushlog models

- **pushlog/** - PushLog models

shtrom · 2026-03-05T05:11:22Z

README.md

+
+### Landing Workers
+
+Separate Docker containers (`hg-landing-worker`, `git-landing-worker`) run Django management commands that loop and poll the database for jobs. Each worker has a corresponding `Worker` model record with configuration details and runs a specific job type.


There's the workers and the compute instance (pod) they run on.

It might be worthwhile mentioning that each worker needs a backing record in the DB in the DB, and that they are then started on the compute instance by specifying the name of the record. Also that there can be multiple workers of a given type, that can be associated to a subset of the Repos for, e.g., permissions granularity.

shtrom · 2026-03-05T05:17:39Z

README.md

+make format                                  # Run ruff, black, and djlint
+make migrations                              # Generate Django migrations
+make upgrade-requirements                    # Upgrade packages in requirements.txt
+make add-requirements                        # Update requirements.txt with new requirements


Suggested change

make add-requirements # Update requirements.txt with new requirements

make add-requirements # Update requirements.txt with new requirements hashes based on the contents of the pyproject.toml

shtrom · 2026-03-05T05:20:16Z

README.md

+`<module name>: <brief description> (bug <bug number>)`

-### Add requirements
+Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bug numbers are required for code changes, and suggest to file a bug if a developer has not provided a bug number.


s/the author should be asked to file/the author should first file/

project: add a CLAUDE.md file (Bug 2015621)

3d890dd

Add a `CLAUDE.md` file which outlines some basic details of working in the Lando project, including some code conventions and general layout of the project. We can start with this as a baseline and iterate on it as needed.

cgsheeh requested a review from a team as a code owner February 9, 2026 21:03

shtrom approved these changes Feb 10, 2026

View reviewed changes

zzzeid reviewed Feb 10, 2026

View reviewed changes

zzzeid requested changes Feb 10, 2026

View reviewed changes

move things not specific to agents into the readme, and make claude.m…

15569c0

…d point to readme

include references to files with versions instead of actual versions

934d67e

cgsheeh added 2 commits February 10, 2026 10:40

add a note about type hints and using explicit types

e986d8a

Merge remote-tracking branch 'origin/main' into claudemd

938878c

cgsheeh requested a review from zzzeid February 10, 2026 16:33

cgsheeh added 2 commits March 3, 2026 18:08

Merge remote-tracking branch 'origin/main' into claudemd

5869715

remove CLAUDE.md

afde008

cgsheeh changed the title ~~project: add a CLAUDE.md file (Bug 2015621)~~ readme: add architecture documentation and coding conventions (Bug 2020842) Mar 3, 2026

cgsheeh requested a review from shtrom March 3, 2026 23:21

cgsheeh requested review from zzzeid and removed request for zzzeid March 3, 2026 23:21

zzzeid requested changes Mar 4, 2026

View reviewed changes

shtrom reviewed Mar 5, 2026

View reviewed changes


		## Coding Conventions

		- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.

	- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.
	- Type-hint function arguments and return values
	- Always add assert messages in tests: `assert a == b, "a and b should be equal"`. Where possible, comments surrounding assert statements should instead become assert messages.


		### Landing Workers

		Separate Docker containers (`hg-landing-worker`, `git-landing-worker`) run Django management commands that loop and poll the database for jobs. Each worker has a corresponding `Worker` model record with configuration details and runs a specific job type.


		### Authentication

		OIDC via Auth0 (`mozilla_django_oidc`) with Django model backend fallback. API authentication via `ApiToken` model.

	OIDC via Auth0 (`mozilla_django_oidc`) with Django model backend fallback. API authentication via `ApiToken` model.
	Landed uses OIDC for authentication via Auth0 (`mozilla_django_oidc`) and Django's `ModelBackend` as a fallback. There are additional authentication mechanisms via Phabricator API tokens and the `lando.headless_api.models.tokens.ApiToken` model used in other parts of the code.


		### Frontend

		Jinja2 templates (primary) with Django Compressor for asset pipeline. Uses Bulma (version in `package.json`) and FontAwesome (vendored in `src/lando/static_src/legacy/vendor/`) — use APIs from the specific versions pinned in those files.

-Jinja2 templates (primary) with Django Compressor for asset pipeline. Uses Bulma (version in `package.json`) and FontAwesome (vendored in `src/lando/static_src/legacy/vendor/`) — use APIs from the specific versions pinned in those files.
+Lando uses the Jinja2 templates backend for various user-facing pages. Django Compressor is used for the for asset pipeline both locally (on-demand) and remotely (at deploy time). Lando uses Bulma as a CSS framework and FontAwesome for fonts. For development purposes, check the corresponding Bulma and FontAwesome APIs based on the pinned versions.
+NOTE: Bulma version can be found in `package.json`, and FontAwesome is checked into the repo in `src/lando/static_src/legacy/vendor/`.

		PostgreSQL (version in `compose.yaml`). Models inherit from `BaseModel` (provides `created_at`/`updated_at`). Migrations live in each app's `migrations/` directory and auto-run on `docker compose up`.

	- Comments in proper English with full punctuation, for example `# this shouldnt happen` should be `# This shouldn't happen.`.
	- Added comments should have correct grammar with full punctuation. For example `# this shouldnt happen` should be `# This shouldn't happen.`.

	- Linting: ruff (line-length 88, isort, annotations) + black + djlint (jinja profile).
	- Ruff and black, are used to enforce Python code style requirements (see ruff.toml), and djlint is used to enforce Jinja2 code style requirements.

	Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bug numbers are required for code changes, and suggest to file a bug if a developer has not provided a bug number.
	Example: `ui: adjust approval banner (bug 1234567)`. Omit the bug number if one hasn't been provided. Note that bugs are required for code changes. If a bug is not associated with a code change, the author should be asked to file a bug and reference it in the pull request.

	Commit messages should include a description of the change being made and some context for why the change was made. This added context allows the reviewer to assert the author's expectations and mental models match the actual code changes.
	Commit messages should include a description of the change being made and some context for why the change was made. This added context allows the reviewer to assert the author's expectations and that the mental models match the actual code changes.

	- pushlog/ - Mercurial pushlog models
	- pushlog/ - PushLog models

	make add-requirements # Update requirements.txt with new requirements
	make add-requirements # Update requirements.txt with new requirements hashes based on the contents of the pyproject.toml

Conversation

cgsheeh commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 9, 2026

Uh oh!

shtrom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzzeid left a comment

Choose a reason for hiding this comment

Uh oh!

cgsheeh commented Feb 10, 2026

Uh oh!

cgsheeh commented Feb 10, 2026

Uh oh!

zzzeid commented Feb 11, 2026

Uh oh!

cgsheeh commented Feb 11, 2026

Uh oh!

cgsheeh commented Mar 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shtrom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cgsheeh commented Feb 9, 2026 •

edited

Loading