Repository layout, commands, and coding conventions are documented in
AGENTS.md and the per-package AGENTS.md files under packages/.
Read those for context; this file covers only what is specific to reviewing a pull
request.
Note that this is a monorepo. Paths below are given in full, because the same filename can exist in more than one package.
- Style: Suggest
npm run format(from the repo root) ornpm run lint-fixfor formatting issues; do not comment on individual style nits. - Patterns: Prefer established Blockly patterns and the official docs over newly invented conventions. This covers conventions — naming, API shape, file layout — not the quality of surrounding code. Do not ask an author to reproduce a nearby antipattern for the sake of consistency, particularly in tests.
- Documentation: Prefer linking to the Blockly docs over
duplicating content in comments. Flag any new link to
developers.google.com/blockly; that site is stale. Existing ones are legacy and can be left alone unless the PR is already touching them. - TSDoc: Public APIs require TSDoc for behavior, params, and returns. Do not include implementation details or historical context unless essential.
- Spelling:
colour,neighbour, andcentreare spelled the British way throughout the public API (setColour,bumpNeighbours,Align.CENTRE). Every other word is American —initialize,serialize,behavior. CSS properties and values are always American (color,background-color,gray). - Dependencies: Flag every new dependency.
packages/blocklyhas no runtime dependencies and must not gain one. A newdevDependencyanywhere needs its justification in the pull request description. - Non-null assertions: Flag new
!assertions and suggest?.or an explicit guard instead. Existing uses are legacy and are not precedent. - Accessibility: Changes that touch the DOM must work for screen reader and
keyboard-only users. Look for keyboard reachability, an accessible name, and use of
Blockly.utils.ariarather than hand-writtenaria-*attributes.
RaspberryPiFoundation/blockly is the canonical upstream, maintained by the Raspberry
Pi Foundation. It is not a fork of google/blockly. Do not suggest changes on the basis
that this is a fork, that upstream should be consulted, or that a change belongs in a
Google-maintained repository.
Copyright <year> Google LLC headers on existing files are correct and historical —
never ask for them to be changed. Only new files use the Raspberry Pi Foundation
copyright. See AGENTS.md.
The description should be concise: what changed, why, and anything a reviewer genuinely needs, such as breaking changes or how to verify non-obvious behavior.
Do not ask authors to expand a description that already covers those things. A short, complete description is correct and preferable. In particular, do not request a rationale narrative, a file-by-file walkthrough, or a restatement of the diff.
Do flag a description that is genuinely missing the why, or one that omits a breaking change.
None of these are hand-written, so do not review their contents line by line or ask for edits to them directly. If one looks wrong, the fix belongs in its source.
The question to ask is not "is this file generated?" but "should this file be in this pull request at all, and does it look regenerated rather than hand-edited?"
| File | Regenerated by |
|---|---|
packages/blockly/msg/json/* |
npm run messages |
package-lock.json |
npm |
Do not nitpick their contents. Do flag them if they look hand-edited rather than
regenerated — for example msg/json/en.json changing without a corresponding change to
packages/blockly/msg/messages.js, an edit to a non-English locale file (those come from
TranslateWiki), or a lockfile change in a PR that touches no dependencies. In those cases
ask for the file to be regenerated, not patched.
| File | Why |
|---|---|
**/CHANGELOG.md |
Written by Lerna at release time, not in feature PRs |
packages/docs/docs/reference/** |
Generated from core TSDoc; gitignored |
**/dist/**, **/build/** |
Build output; gitignored |
A change to any of these in a normal pull request means something has gone wrong, and is
worth a comment. A hand-written CHANGELOG.md entry in particular should be flagged:
release notes come from conventional commit messages, so the fix is to write a good
commit message rather than to edit the changelog. The last three are gitignored
entirely, so their presence means they were force-added.
The exception is a release pull request, where these files changing is the whole point.
- All user-visible strings must use
Blockly.Msg, including ARIA labels and other screen-reader-only text. - This applies to
packages/plugins/*too. Plugins have no message files of their own, so a plugin pull request that adds a string will correctly also changepackages/blockly/msg/. Do not flag that as unrelated scope. - New strings are hand-written only in
packages/blockly/msg/messages.js.packages/blockly/msg/json/en.jsonandpackages/blockly/msg/json/qqq.jsonare then regenerated bynpm run messages. - A pull request that adds a string should therefore contain changes to all three files.
If
en.jsonorqqq.jsonis missing, the author likely forgot to runnpm run messages; if they were edited by hand, ask for them to be regenerated instead. - Link the localization guide if strings are missing or misplaced.
- PRs that attempt to add translations for non-English strings should be redirected to TranslateWiki, using the same guide.
- Test files must be named
*.mocha.js. The plugin test runner silently finds no tests and exits successfully if they are named anything else, so a green CI run does not prove the tests ran. Flag any new plugin test file that does not use this suffix. - New plugins must be TypeScript. Flag a newly added plugin written in JavaScript;
the scaffolding generator defaults to JS, so this is an easy one to miss
(
npx @blockly/create-package plugin <name> --typescript). Existing JavaScript plugins are legacy — do not ask for them to be converted. blocklymust be a peer dependency, never a direct dependency. Flag imports that reach into deep paths inside the core package rather than its public entry points.- Package names follow the type-based convention in the plugin naming guide. Flag new packages that do not follow it.
- Hand-edited
versionfields orCHANGELOG.mdentries should be flagged; both are managed by Lerna.
- The definition of a breaking change, and the full list of what does and does not count,
is in
packages/blockly/AGENTS.md. Use that list rather than a separate one here, so the two cannot drift apart. - Prefer deprecation with migration paths over removal.
- Compatibility: Must support Safari 15.4+, latest Chrome, and latest Firefox.
Flag breaking changes unless all of the following are true:
- The PR description explicitly notes it.
- The commit type includes
!(e.g.,feat!:). - The target branch is not
main.
All three are required. Breaking changes are never merged straight to main: they land
on a version branch (for example v14) and are merged to main together when that
version is released. So a properly labelled breaking change that still targets main is
worth flagging — it is almost always aimed at the wrong branch.