Conversation
It’s time we start formalizing our relationship with Claude :)
There was a problem hiding this comment.
Pull request overview
Adds a repository-level guidance file intended for Claude Code to follow this repo’s development workflow and architectural conventions, and ensures it’s no longer ignored by git.
Changes:
- Add
CLAUDE.mdwith setup/tooling/architecture guidance for the Teams Python SDK monorepo. - Update
.gitignoreto stop ignoringCLAUDE.md.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
CLAUDE.md |
Introduces Claude-specific repo guidance (setup, tooling, architecture, patterns). |
.gitignore |
Removes the ignore rule so CLAUDE.md is tracked in the repo. |
You can also share your feedback on Copilot code review. Take the survey.
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. |
There was a problem hiding this comment.
The PR description mentions initializing a ".claude.md", but this change adds/uses a root-level "CLAUDE.md" (and removes it from .gitignore). If the intended filename is dot-prefixed, rename the file accordingly; otherwise update the PR title/description to match the actual filename to avoid confusion for contributors.
| ## Development Setup | ||
|
|
||
| ### Prerequisites | ||
| - UV >= 0.9.8 |
There was a problem hiding this comment.
The stated UV prerequisite (">= 0.9.8") doesn’t match the repo’s existing guidance and automation: README.md lists UV ">= 0.8.11", and the CI workflow installs uv 0.5.24. Please align this prerequisite with the repo’s documented/supported version (or avoid pinning to a higher minimum) so contributors don’t assume they need a version newer than what CI uses.
| - UV >= 0.9.8 | |
| - UV >= 0.8.11 |
|
|
||
| - **Formatter/Linter**: Ruff — line length 120, rules: E, F, W, B, Q, I, ASYNC | ||
| - **Type checker**: Pyright | ||
| - **Test framework**: pytest + pytest-asyncio (ruff bans unittest) |
There was a problem hiding this comment.
“ruff bans unittest” is misleading in this repo: the Ruff config only bans importing the unittest framework module, while the test suite uses unittest.mock in many places. Consider rewording this to clarify that unittest.TestCase-style tests are discouraged, but unittest.mock is acceptable/used.
| - **Test framework**: pytest + pytest-asyncio (ruff bans unittest) | |
| - **Test framework**: pytest + pytest-asyncio (Ruff bans importing the unittest test framework; unittest.mock is allowed and used) |
| ## Architecture | ||
|
|
||
| ### Workspace Structure | ||
| All packages live in `packages/`, each with `src/microsoft_teams/<package>/` layout: |
There was a problem hiding this comment.
The workspace layout description is incomplete: each package includes both src/microsoft_teams/... (current namespace) and src/microsoft/teams/... (backward-compat shim namespace). Updating this line to mention the shim path will prevent contributors from deleting or misinterpreting the src/microsoft tree.
| All packages live in `packages/`, each with `src/microsoft_teams/<package>/` layout: | |
| All packages live in `packages/`, each with both `src/microsoft_teams/<package>/` (current namespace) and `src/microsoft/teams/<package>/` (backward-compat shim namespace) layout: |
| - ALL imports MUST be at the top of the file — no imports inside functions, classes, or conditional blocks | ||
| - Avoid `TYPE_CHECKING` blocks unless absolutely necessary (genuine circular imports that can't be restructured) | ||
| - Avoid dynamic/deferred imports unless absolutely necessary | ||
| - Relative imports within the same package, absolute for external packages |
There was a problem hiding this comment.
The “Imports” rules are stricter than the codebase reality and will conflict with existing patterns: the repo uses TYPE_CHECKING blocks (e.g., forward refs/optional deps) and sometimes does lazy/conditional imports (e.g., optional Graph dependency). Please soften these bullets to reflect the actual convention (prefer top-level imports, but allow TYPE_CHECKING and local imports when needed for optional deps or circular-import avoidance).
| - ALL imports MUST be at the top of the file — no imports inside functions, classes, or conditional blocks | |
| - Avoid `TYPE_CHECKING` blocks unless absolutely necessary (genuine circular imports that can't be restructured) | |
| - Avoid dynamic/deferred imports unless absolutely necessary | |
| - Relative imports within the same package, absolute for external packages | |
| - Prefer imports at the top of the file; local/conditional imports are allowed when needed for optional dependencies, performance, or to avoid circular imports | |
| - `TYPE_CHECKING` blocks are allowed for forward references, optional dependencies, or genuine circular-import issues; otherwise keep imports regular and top-level | |
| - Prefer simple, eager imports; use dynamic/deferred imports only for optional features (e.g., Graph) or heavy dependencies that are not always needed | |
| - Use relative imports within the same package and absolute imports for external packages |
6e6f9fc to
ce25640
Compare
It’s time we start formalizing our relationship with Claude :). This PR initializes a simple .claude.md with some common rules that claude tends to get wrong often.