Skip to content

Commit eeb03c3

Browse files
committed
Adds PR review instructions
1 parent 71e4ac5 commit eeb03c3

3 files changed

Lines changed: 87 additions & 8 deletions

File tree

.github/copilot-instructions.md

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Copilot Instructions for CLI for Microsoft 365
22

33
## Project Overview
4+
45
- This is a cross-platform CLI tool to manage Microsoft 365 tenants and SharePoint Framework (SPFx) projects.
56
- Written in TypeScript, targeting Node.js LTS (see `package.json`, `tsconfig.json`).
67
- Main entry: `src/index.ts``src/cli/cli.ts` (command parsing/execution).
@@ -10,6 +11,7 @@
1011
- Command discovery/build: see `scripts/write-all-commands.js` (generates `allCommands.json`).
1112

1213
## Key Patterns & Conventions
14+
1315
- **Global options**: All commands accept `--output`, `--query`, `--debug`, `--verbose` (see `src/GlobalOptions.ts`).
1416
- **Authentication**: Managed via `src/Auth.ts`, supports multiple auth types (Device Code, Certificate, Secret, Managed Identity, etc.).
1517
- **HTTP requests**: Use the wrapper in `src/request.ts` (Axios-based, with logging/debug hooks).
@@ -19,7 +21,67 @@
1921
- **Config**: User settings/configuration via `configstore` (see `src/config.ts`).
2022
- **Completion**: Shell completion logic/scripts in `scripts/Register-CLIM365Completion.ps1` and `scripts/Test-CLIM365Completion.ps1`.
2123

24+
## Command Implementation
25+
26+
- Command class should extend the appropriate workload-specific base class (`SpoCommand`, `GraphCommand`, `AzmgmtCommand`, etc.) when applicable; otherwise, it may extend `Command` directly.
27+
- Class name must follow the `[Service][Entity][Action]Command` pattern (e.g., `SpoGroupGetCommand`).
28+
- Must implement `name`, `description`, and `commandAction()` at minimum.
29+
- Options must be defined using **Zod schemas** with `z.strictObject({ ...globalOptionsZod.shape, ... })` or `globalOptionsZod.strict()`.
30+
- Option names must be camelCase; apply aliases with the schema `.alias('x')` method (for example, `.alias('u')` for `--webUrl` where appropriate).
31+
- Commands that delete/remove resources must include a `--force` option.
32+
- Use `async/await`, never `.then()` chains.
33+
- Use `logger.logToStderr()` for verbose/debug messages, never `logger.log()`.
34+
- Add verbose log messages where they provide useful execution context; every command requires at least one verbose logging statement.
35+
- Never format output conditionally based on `--output json`; use `defaultProperties` for text field filtering.
36+
- All `commands.ts` files are sorted alphabetically by command name; new commands must be added in the correct order.
37+
38+
## API Usage
39+
40+
- Use `request.ts` wrapper for all HTTP calls, never call APIs directly.
41+
- Use `handleRejectedODataJsonPromise()` for JSON response error handling.
42+
- For SharePoint: use `GetFileByServerRelativePath` / `GetFolderByServerRelativePath`, never `GetFileByServerRelativeUrl` / `GetFolderByServerRelativeUrl`.
43+
- Escape user input in XML payloads and URL parameters using `formatting.encodeQueryParameter()` or `formatting.escapeXml()`.
44+
- Avoid unnecessary form digest retrieval.
45+
46+
## Testing
47+
48+
- Test file must be alongside the command file with `.spec.ts` extension.
49+
- Use Mocha (`describe`/`it`/`before`/`beforeEach`/`afterEach`/`after`) + Sinon (stubs/spies).
50+
- Standard test setup must stub: `auth.restoreAuth`, `telemetry.trackEvent`, `pid.getProcessName`, `session.getId`.
51+
- Must set `auth.connection.active = true` in `before()`.
52+
- Must initialize `commandInfo` and `commandOptionsSchema` in `before()` for Zod-based commands.
53+
- Must include tests for: command name matches constant, description is not null, schema validation (valid and invalid options), success paths, error handling paths.
54+
- Options in `command.action()` calls must be parsed through `commandOptionsSchema.parse()`, not passed as raw objects.
55+
- Never use `as any` to bypass type checking unless absolutely necessary for error path testing.
56+
- Use `sinon.restore()` in `after()` to reset stubs/spies.
57+
58+
## Documentation
59+
60+
- Every command needs a reference page at `docs/docs/cmd/<workload>/<command-name>.mdx`.
61+
- The `.mdx` file name matches the command file name.
62+
- Must include: title, description, usage, options table, examples, permissions, and response.
63+
- The remarks section can be used for additional information but is not mandatory.
64+
- When a command has a response, include sample output for all four output formats: JSON, Markdown, text, and CSV.
65+
- Include at least 2 examples for the examples section, covering different option combinations.
66+
- Import and use `<Global />` for standard CLI options.
67+
- Examples should use `m365` prefix and long option names (not short aliases).
68+
- Document minimum required permissions that allow success with any option combination.
69+
- Docs must build without warnings.
70+
- When importing components in the docs, use absolute imports from `@site/src/components/` instead of relative paths.
71+
- When importing global options in the docs, use a relative import from `../_global.mdx`.
72+
73+
## Code Quality
74+
75+
- No `any` types — use proper TypeScript interfaces/types.
76+
- No commented-out code.
77+
- Single quotes for all strings, where possible.
78+
- No unused imports or variables.
79+
- Follow existing patterns in neighboring command files for consistency.
80+
- Custom ESLint rules are enforced: command class naming, command name dictionary, no deprecated API usage.
81+
- New command name words may require adding to the ESLint dictionary in `eslint.config.mjs`.
82+
2283
## Developer Workflows
84+
2385
- **Build**: `npm run build` (TypeScript compile + command metadata generation)
2486
- **Test**: `npm test` (runs version check, lint, and Mocha tests)
2587
- **Lint**: `npm run lint` (uses custom ESLint rules from `eslint-rules/`)
@@ -29,25 +91,25 @@
2991
- **Docs**: Docusaurus site in `docs/` (config: `docs/docusaurus.config.ts`)
3092

3193
## Project-Specific Notes
32-
- **Command structure**: Each command is a class, not a function. Use inheritance from `Command`.
94+
3395
- **Command registration**: New commands must be discoverable for `write-all-commands.js` to pick up.
3496
- **SPFx support**: Special logic for SPFx project upgrades and compatibility checks in `src/m365/spfx/`.
3597
- **Output**: Prefer returning objects/arrays; formatting handled by CLI core.
36-
- **No direct file/console output in commands**: Use provided logger and output mechanisms.
37-
- **Testing**: Mocha-based, see test files alongside source (e.g., `*.spec.ts`). 100% code coverage.
38-
- **Custom ESLint rules**: See `eslint-rules/` and `eslint-plugin-cli-microsoft365` in `package.json`.
3998

4099
## Integration & External Dependencies
100+
41101
- **Microsoft Graph, SharePoint REST, etc.**: Use `request.ts` for all HTTP calls.
42102
- **Authentication**: Uses `@azure/msal-node` and related packages.
43103
- **Telemetry**: Application Insights via `applicationinsights` package.
44104
- **Docs**: Docusaurus, see `docs/` and `docs/docusaurus.config.ts`.
45105

46106
## Examples
107+
47108
- Add a new command: Get the latest information about building commands from [the docs](../docs/docs/contribute/new-command/build-command-logic.mdx). Create a class in the appropriate `src/m365/<workload>/commands/` folder, extend `Command`, implement `commandAction`, register options, and ensure it is discoverable. Create a test file next to the command file. Create a reference page in the documentation in `docs/docs/cmd/<workload>`. The reference page file name is the same as the command file name, but with the `.mdx` extension.
48109
- Add a global option: update `src/GlobalOptions.ts` and propagate to command parsing in `src/cli/cli.ts`.
49110

50111
## References
112+
51113
- [Main README](../README.md)
52114
- [Contributing Guide](../docs/docs/contribute/contributing-guide.mdx)
53115
- [Docs site](https://pnp.github.io/cli-microsoft365/)
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
description: "Use when reviewing a pull request, providing PR feedback, or checking code quality for CLI for Microsoft 365 contributions."
3+
---
4+
5+
# Pull Request Review Guidelines for CLI for Microsoft 365
6+
7+
When reviewing a PR, verify each of the following areas. For detailed coding conventions (command implementation, API usage, testing, documentation, and code quality), refer to `.github/copilot-instructions.md`.
8+
9+
## PR Structure
10+
11+
- PR title should be descriptive (e.g., "Adds 'spo site get' command")
12+
- PR should reference the issue it addresses (e.g., "Closes #1234")
13+
- PR should target the `main` branch only
14+
- New command PRs should include changes to: command file, spec file, `commands.ts`, docs `.mdx` file, and `docs/src/config/sidebars.ts`
15+
16+
## Review Checklist
17+
18+
- For bug fixes: include a test that reproduces the original bug

docs/docs/contribute/pr-checklist.mdx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ This document outlines the requirements for submitting a pull request (PR) to th
1818
- [ ] The command class is named following the pattern `[Service][Command]Command`. For example, `SpoWebRemoveCommand`.
1919
- [ ] Verify the command works as expected.
2020
- [ ] List commands must have readable output in `text` mode, with each item fitting in one row of 130 characters preferably.
21-
- [ ] Command telemetry must not track Personally Identifiable Information (PII).
2221
- [ ] Avoid commented-out code and usage of `any` types, preferring specific types.
2322
- [ ] Remove commands should include a `force` option.
2423
- [ ] For bug fixes, include a test for the fixed use case.
@@ -27,17 +26,17 @@ This document outlines the requirements for submitting a pull request (PR) to th
2726
- [ ] Escape user input in XML and URLs.
2827
- [ ] Verbose and debug outputs are logged to stdErr (`logger.logToStderr` instead of `logger.log`).
2928
- [ ] Do not do conditional output in JSON output mode; use `defaultProperties` for defining default properties.
30-
- [ ] For commands with multiple options where the user is required to choose one, define these options using an `optionSet` without specific validation logic in the command's validate method.
3129
- [ ] Use `async/await` instead of `promise/then`.
3230
- [ ] When working with `spo` commands, use `GetFileByServerRelativePath` and `GetFolderByServerRelativePath` API endpoint instead of `GetFileByServerRelativeUrl` and `GetFolderByServerRelativeUrl`.
3331
- [ ] All tests must pass.
3432

3533
## Documentation
3634

37-
- [ ] Include an md help page.
38-
- [ ] Reference the md help page in the table of contents.
35+
- [ ] Include an `.mdx` help page.
36+
- [ ] Reference the `.mdx` help page in the table of contents.
3937
- [ ] Start all code samples with `m365`.
4038
- [ ] Ensure samples use long names of options rather than short ones.
4139
- [ ] Include the marker to incorporate global options rather than listing them explicitly.
4240
- [ ] Check for no warnings when building docs (lines that begin with `WARNING - `).
4341
- [ ] If there is an option modifying the output, include responses for both default and modified output.
42+
- [ ] For a command page, ensure it includes a title, description, usage, options table, examples, permissions, and response sections.

0 commit comments

Comments
 (0)