Skip to content

Commit 779cf87

Browse files
authored
chore: update PR remplate, labels & copilot instructions (#7593)
* chore: update PR remplate & copilot instructions * chore: add proper labels * chore: use commit instead of master for sonarcloud * chore: update labeler
1 parent cbce07c commit 779cf87

File tree

11 files changed

+407
-17
lines changed

11 files changed

+407
-17
lines changed

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,29 @@
1-
## Description
1+
# Pull Request
22

3-
<!-- Uncomment this section if your PR has UI changes -->
4-
<!--
5-
## Screenshots/Screencast (for UI changes)
6-
-->
3+
## Summary
74

8-
## Checklist
5+
- What did I change and why?
6+
- Risks and how to roll out / roll back (e.g. feature flags):
97

10-
- [ ] mentions the JIRA issue in the PR name (Ex. [WPB-XXXX])
11-
- [ ] PR has been self reviewed by the author;
12-
- [ ] Hard-to-understand areas of the code have been commented;
13-
- [ ] If it is a core feature, unit tests have been added;
8+
---
149

15-
<!-- Uncomment this section if it is necessary to understand the PR -->
16-
<!-- ## Important Details for the Reviewers
10+
## Security Checklist (required)
1711

18-
- use (x) data
19-
- can be reviewed commit-by-commit
20-
- be sure to look at ... -->
12+
- [ ] **External inputs are validated & sanitized** on client and/or server where applicable.
13+
- [ ] **API responses are validated**; unexpected shapes are handled safely (fallbacks or errors).
14+
- [ ] **No unsafe HTML is rendered**; if unavoidable, sanitization is applied **and** documented where it happens.
15+
- [ ] **Injection risks (XSS/SQL/command) are prevented** via safe APIs and/or escaping.
16+
17+
## Standards Acknowledgement (required)
18+
19+
- [ ] I have read and this PR **upholds** our [Coding Standards](https://github.com/wireapp/wire-web-packages/tree/docs/coding-standards.md) and [Tech Radar Choices](https://github.com/wireapp/wire-web-packages/tree/docs/tech-radar.md).
20+
21+
---
22+
23+
## Screenshots or demo (if the user interface changed)
24+
25+
## Notes for reviewers
26+
27+
- Trade-offs:
28+
- Follow-ups (linked issues):
29+
- Linked PRs (e.g. web-packages):

.github/copilot-instructions.md

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Copilot — Repository Instructions (Security-first)
2+
3+
## Goals & tone
4+
5+
- Apply our **Web Coding Standards** during **PR description drafting** and **code review**.
6+
- Prefer **specific, actionable** comments with short code examples.
7+
- Use severities: **[Blocker]**, **[Important]**, **[Suggestion]**.
8+
- Do **not** nitpick items handled by automation (formatting, lint rules).
9+
10+
Related docs (important):
11+
12+
- **Coding Standards:** docs/coding-standards.md
13+
- **Tech Radar:** docs/tech-radar.md
14+
15+
**Additional rules (open when relevant)**
16+
17+
- Security: `.github/instructions/security.instructions.md`
18+
- Accessibility: `.github/instructions/accessibility.instructions.md`
19+
- React/UX: `.github/instructions/react.instructions.md`
20+
- TypeScript: `.github/instructions/typescript.instructions.md`
21+
22+
---
23+
24+
## PR description — Auto-checks Copilot should perform
25+
26+
---
27+
28+
## Security Checklist (required)
29+
30+
- [ ] **External inputs validated & sanitised** (client/server as applicable). _Tick if_ validation/sanitisation is visible before use.
31+
- [ ] **API responses validated**; unexpected shapes handled (fallbacks/errors). _Tick if_ guards/schemas are present at boundaries.
32+
- [ ] **No unsafe HTML is rendered**; if unavoidable, sanitisation is applied **and** noted where it happens. _Fail signal:_ `dangerouslySetInnerHTML` without sanitiser (e.g., `DOMPurify.sanitize`).
33+
- [ ] **Injection risks (XSS/SQL/command) mitigated** via safe APIs/escaping. _Tick if_ sinks are avoided or safely wrapped.
34+
35+
---
36+
37+
## When reviewing pull requests (Copilot)
38+
39+
**Scope & approach**
40+
41+
- Review **from the code diff only**; do not assume runtime behavior.
42+
- Use severities: **[Blocker]**, **[Important]**, **[Suggestion]**.
43+
- Provide concise, actionable comments; include a minimal before/after snippet where useful.
44+
45+
### Security (focus of inline review)
46+
47+
- Avoid/flag `dangerouslySetInnerHTML`; if present, require sanitisation and name the sanitizer.
48+
- No raw DOM insertion into trusted contexts; validate URLs/redirect targets.
49+
- Validate untrusted inputs and API responses **before** use; prefer schemas/guards.
50+
- Check for secrets/tokens in code, configs, and tests.
51+
- Call out missing error/fallback paths on boundary failures.
52+
53+
### Accessibility (minimum check)
54+
55+
- Keyboard access (Esc closes dialogs), visible focus, correct roles/labels.
56+
- Use of `aria-live` for async status where appropriate.
57+
58+
### Everything else
59+
60+
- For imports/TS/React/testing/naming/readability: **refer to** the [Coding Standards](docs/coding-standards.md).
61+
- If a standard is violated, link the relevant section and suggest a minimal change.
62+
63+
### Technology choices
64+
65+
- Compare any new dependencies in `package.json`/lockfiles to the [Tech Radar](docs/tech-radar.md).
66+
- If not **Adopt**/**Trial**, mark **[Blocker]** and request an RFC/approval link.
67+
- For **Trial**, ensure usage is narrowly scoped and success criteria exist.
68+
69+
---
70+
71+
## Comment format Copilot should use
72+
73+
**Top-level summary**
74+
75+
- Verdict: **Ready** / **Changes requested**, with counts of Blockers/Important/Suggestions.
76+
- Mini checklist (only items evidenced by diff): Security, Accessibility, Tech choices, (then link to Coding Standards for any non-security notes).
77+
78+
**Inline comments**
79+
80+
- One issue per comment with severity, file:line, brief reason, and (when helpful) a minimal suggested patch.
81+
82+
**Approval**
83+
84+
- Approve only if there are **no Blockers** and Important items are fixed or explicitly deferred with rationale.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
appliesTo:
3+
paths:
4+
- 'src/**/*'
5+
---
6+
7+
# Copilot — Accessibility
8+
9+
- Ensure keyboard access is coded: focusable controls; Escape key closes dialogs; trap & restore focus in dialogs.
10+
- Use semantic elements; provide names/labels; add `aria-live` for async status updates where relevant.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
appliesTo:
3+
paths:
4+
- 'src/**/*.tsx'
5+
---
6+
7+
# Copilot — React code review rules
8+
9+
- Prefer event handlers or a data tool over `useEffect`.
10+
- If an effect is necessary, it must: (1) do one thing, (2) have stable deps, (3) avoid inline objects/functions in deps.
11+
- No derived state from props unless justified.
12+
- Keys in lists: stable unique ids; never array indexes.
13+
- Keep UI components “dumb”: render from props; move business logic to a separate module or hook.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
appliesTo:
3+
paths:
4+
- 'src/**/*'
5+
---
6+
7+
# Copilot — Security hygiene
8+
9+
- Validate and sanitise **all untrusted input** and **all API responses**.
10+
- Avoid `dangerouslySetInnerHTML`. If used, it must be sanitised and commented with the sanitizer reference.
11+
- Avoid raw DOM insertion and unvalidated URLs; suggest safe helpers.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
appliesTo:
3+
paths:
4+
- 'src/**/*.ts'
5+
- 'src/**/*.tsx'
6+
---
7+
8+
# Copilot — TypeScript safety
9+
10+
- Do not use `any`, type casts (`as T`/`<T>`), or the non-null operator `!`.
11+
- Exported APIs must have explicit types. Narrow with type guards (or schemas) at boundaries.
12+
- Handle `null`/`undefined` explicitly; avoid `@ts-ignore` (unless narrowly scoped with a reason).

.github/labeler.yml

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Labels based on PR title
1+
# Types
22
'type: refactoring 🛠':
33
title: '^refactor(\(.+\))?:.*'
44
'type: bug / fix 🐞':
@@ -12,10 +12,53 @@
1212
'type: test 👷':
1313
title: '^test(\(.+\))?:.*'
1414

15+
# Sizes
16+
'👕 size: XS':
17+
size-below: 10
18+
'👕 size: S':
19+
size-above: 9
20+
size-below: 100
21+
'👕 size: M':
22+
size-above: 99
23+
size-below: 300
24+
'👕 size: L':
25+
size-above: 299
26+
size-below: 500
27+
'👕 size: XL':
28+
size-above: 499
29+
size-below: 1000
30+
'👕 size: XXL':
31+
size-above: 999
32+
1533
# Labels based on changed files
1634
'comp: infrastructure':
1735
files:
1836
- 'bin/.*'
1937
- '.github/.*'
2038
- '.travis.yml'
2139
- 'lerna.json'
40+
'comp: api-client':
41+
files:
42+
- 'packages/api-client/.*'
43+
'comp: core':
44+
files:
45+
- 'packages/core/.*'
46+
'comp: ui-kit':
47+
files:
48+
- 'packages/ui-kit/.*'
49+
'comp: store':
50+
files:
51+
- 'packages/store-engine/.*'
52+
- 'packages/store-engine-dexie/.*'
53+
- 'packages/store-engine-fs/.*'
54+
'comp: config':
55+
files:
56+
- 'packages/eslint-config/.*'
57+
- 'packages/prettier-config/.*'
58+
- 'packages/copy-config/.*'
59+
'comp: telemetry':
60+
files:
61+
- 'packages/telemetry/.*'
62+
'comp: events':
63+
files:
64+
- 'packages/webapp-events/.*'
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
name: Link and Lint PR with Jira Ticket Number
2+
on:
3+
pull_request:
4+
types: [opened, edited, synchronize]
5+
jobs:
6+
add-jira-description:
7+
# avoid triggering this action on dependabot PRs
8+
if: ${{ github.actor != 'dependabot[bot]' }}
9+
runs-on: ubuntu-latest
10+
steps:
11+
- uses: cakeinpanic/jira-description-action@235b8296c8b4f88c564297246810f084f853dac1
12+
name: jira-description-action
13+
with:
14+
github-token: ${{ secrets.GITHUB_TOKEN }}
15+
jira-token: ${{ secrets.JIRA_TOKEN }}
16+
jira-base-url: https://wearezeta.atlassian.net
17+
skip-branches: '^(dev|master|release\/*)$'
18+
fail-when-jira-issue-not-found: true

.github/workflows/label.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ jobs:
1111
steps:
1212
# The settings for this action are in `.github/labeler.yml`.
1313
# See https://github.com/srvaroa/labeler.
14-
- uses: srvaroa/labeler@v1.4
14+
- uses: srvaroa/labeler@0a20eccb8c94a1ee0bed5f16859aece1c45c3e55
1515
env:
1616
GITHUB_TOKEN: ${{secrets.OTTO_THE_BOT_GH_TOKEN}}

docs/coding-standards.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Coding Standards
2+
3+
These standards guide everyday code contributions. They complement the Security Checklist in the PR template.
4+
5+
## 1) Imports, Exports, and Module Boundaries
6+
7+
- Use **named exports only**; avoid default exports.
8+
- Use **named imports only**; do not rename imports with `as`.
9+
- Avoid circular dependencies; keep module boundaries clear.
10+
11+
## 2) TypeScript: Type Safety & Validation
12+
13+
- Avoid `any`, type assertions/casts (`as`, `<Type>`) and the non-null `!` operator unless absolutely necessary and justified.
14+
- Prefer type guards for safe narrowing.
15+
- Validate external data (user input, API responses, storage) before use; handle unexpected shapes.
16+
17+
## 3) Naming, Documentation, and Readability
18+
19+
- Use descriptive names; avoid vague terms/abbreviations.
20+
- Add JSDoc-style comments for methods (purpose, parameters, returns).
21+
- Prefer one statement per line (including split chained calls).
22+
- Prefer modern TS/JS features (destructuring, spread/rest).
23+
24+
## 4) Coding Style & Functionality
25+
26+
- Minimize variable scope; declare variables near first use.
27+
- Prefer early returns over deep nesting.
28+
- Keep helper functions pure and side-effect free.
29+
- Prefer arrow functions unless `this/arguments` semantics are required.
30+
- Ensure no unhandled promises.
31+
32+
## 5) React: Effects, State, and Components
33+
34+
- Do **not** reach for `useEffect` by default; trigger actions from events.
35+
- Use declarative data tools instead of hand-wired effects where possible.
36+
- Each effect has a single responsibility with minimal, stable deps.
37+
- Avoid inline objects/functions that cause re-renders.
38+
- Keep components “dumb” and focused—render from props; move logic up.
39+
- Prefer functional components + hooks.
40+
- Use stable keys in lists (never array indexes).
41+
42+
## 6) Abstraction and Duplication
43+
44+
- Duplication is sometimes better than a wrong abstraction.
45+
- Inline first; abstract once patterns stabilize.
46+
- Be willing to delete/inline poor abstractions.
47+
48+
## 7) Accessibility (a11y)
49+
50+
- Full keyboard support; visible focus; Escape closes modals.
51+
- Use appropriate ARIA landmarks/roles; live regions for async updates.
52+
- Custom components follow accessible patterns (e.g., focus-trapped dialogs, proper labeling, sensible tab order).
53+
54+
## 8) Testing
55+
56+
- Component unit tests for new/changed logic (cover success & failure paths).
57+
- E2E tests for affected critical paths, or mark N/A with a reason.
58+
- For TS modules, include meaningful unit tests; emphasize public APIs; prefer pure functions.
59+
60+
## 9) Technology Choices ([Tech Radar](./tech-radar.md))
61+
62+
- Use well-understood, approved technologies.
63+
- If introducing experimental tech, keep usage limited, justify it.
64+
- Do not introduce unrecommended tech; prefer refactoring/migrating away.
65+
66+
## 10) Documentation
67+
68+
- Update documentation when behavior or usage changes.
69+
- Document migrations.
70+
- Keep PR title/description clear and consistent with the change.
71+
- Link the PR to the relevant Jira ticket.
72+
73+
## 11) PR Formatting & Media
74+
75+
- For UI/design changes, include screenshots or a short video.
76+
- For technical PRs, include a clear description of the problem, scope, and user impact.
77+
- Add inline comments on critical paths/logic to guide reviewers.
78+
79+
---
80+
81+
**Note:** Security-critical checks live in the PR template’s “Security Checklist” and must be explicitly acknowledged there.

0 commit comments

Comments
 (0)