|
| 1 | +# Code review guidelines |
| 2 | + |
| 3 | +## General principles |
| 4 | +- **Style:** Suggest `npm run format` or `npm run lint:fix` for formatting issues; do not comment on individual style nits. |
| 5 | +- **Patterns:** Enforce existing Blockly patterns and official docs over new conventions. |
| 6 | +- **Documentation:** Prefer linking to [Blockly Dev Docs](https://developers.google.com/blockly) over duplicating content in comments. |
| 7 | +- **TSDoc:** Public APIs require TSDoc for behavior, params, and returns. Do not include implementation details or historical context unless essential. |
| 8 | + |
| 9 | +## Localization |
| 10 | +- All user-visible strings must use `Blockly.Msg`. |
| 11 | +- New strings must be added to `msg/messages.js`, `msg/json/qqq.json`, and `msg/json/en.json`. |
| 12 | +- Link [this guide](https://developers.google.com/blockly/guides/contribute/core/add_localization_token) if strings are missing or misplaced. |
| 13 | +- PRs that attempt to add translations for non-English strings should be redirected to TranslateWiki via the ([translation guide](hhttps://developers.google.com/blockly/guides/contribute/core/translating)). |
| 14 | + |
| 15 | +## Breaking changes |
| 16 | +### Policy |
| 17 | +- A breaking change is any non-backwards-compatible change to public APIs, behavior, UI, or browser requirements. |
| 18 | +- **Avoid:** Prefer deprecation with migration paths over removal. |
| 19 | +- **Compatibility:** Must support Safari 15.4+, latest Chrome, and latest Firefox. |
| 20 | +- **Identification:** Flag breaking changes unless all of the following are true: |
| 21 | + 1. PR description explicitly notes it. |
| 22 | + 2. Commit type includes `!` (e.g., `feat!:`). |
| 23 | + 3. Target branch is not `main`. |
| 24 | + |
| 25 | +### Breaking |
| 26 | +- Removing/renaming public methods, properties, or classes. |
| 27 | +- Changing signatures or behavior of existing public methods. |
| 28 | +- Adding required methods to public interfaces. |
| 29 | +- New keyboard shortcuts or context menu items (potential developer conflicts). |
| 30 | +- DOM restructures affecting external CSS/JS. |
| 31 | +- Changes to build output/consumption (e.g., ESM-only). |
| 32 | +- Changes that affect the output of serialization. |
| 33 | + |
| 34 | +### Non-breaking (do not flag) |
| 35 | +- Additive changes (new methods/properties). |
| 36 | +- Internal refactoring (including items marked `@internal`). |
| 37 | +- Tooling/workflow changes. |
| 38 | +- Changes to unreleased code (non-`main` feature branches). |
0 commit comments