Conversation
AGENTS.md
Outdated
|
|
||
| ## Type Checking | ||
|
|
||
| - Always run mypy from the main `.venv`. If there is no `.venv`, create it and install `pip install -r requirements-lint.txt -r requirements-test.txt` |
There was a problem hiding this comment.
We should create a tox env that only runs mypy imo, feel like it's easy for a virtual env to be polluted.
There was a problem hiding this comment.
I like the idea. Added ruff and mypy as tox envs and tweaked the file.
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Documentation 📚
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 4.57s All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 13794 uncovered lines. Generated by Codecov Action |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| [testenv:mypy] | ||
| commands = | ||
| mypy sentry_sdk | ||
|
|
||
| [testenv:ruff] | ||
| commands = | ||
| ruff check tests sentry_sdk | ||
| ruff format --check tests sentry_sdk |
There was a problem hiding this comment.
Bug: Manual changes to tox.ini for mypy and ruff environments will be lost when the auto-generation script populate_tox.py is run, as it overwrites the file.
Severity: MEDIUM
Suggested Fix
Instead of editing tox.ini directly, add the [testenv:mypy] and [testenv:ruff] configurations to the source template file, scripts/populate_tox/tox.jinja. This will ensure the changes are preserved when the auto-generation script runs.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: tox.ini#L942-L949
Potential issue: The `tox.ini` file is being manually modified to add `[testenv:mypy]`
and `[testenv:ruff]` sections. However, a comment at the top of `tox.ini` warns that the
file is auto-generated by `scripts/populate_tox/populate_tox.py` from the `tox.jinja`
template. The generation script opens `tox.ini` in write mode (`"w"`), which overwrites
the entire file. Since the new `mypy` and `ruff` environments are not present in the
`tox.jinja` template, these manual changes will be lost the next time the generation
script is executed. This will break the `tox -e mypy` and `tox -e ruff` commands
documented in `AGENTS.md`.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| [testenv:ruff] | ||
| commands = | ||
| ruff check tests sentry_sdk | ||
| ruff format --check tests sentry_sdk |
There was a problem hiding this comment.
Direct edits to auto-generated file will be lost
Medium Severity
The new [testenv:mypy] and [testenv:ruff] environments, along with their mypy: and ruff: factor-conditional deps and basepython entries, were added directly to tox.ini. However, tox.ini is auto-generated from scripts/populate_tox/tox.jinja, and the file itself begins with # DON'T EDIT THIS FILE BY HAND. tox.jinja has no corresponding changes, so the next run of scripts/generate-test-files.sh will silently overwrite and delete these new environments, making the tox -e mypy and tox -e ruff commands described in AGENTS.md disappear.
Additional Locations (1)
|
|
||
| [testenv:mypy] | ||
| commands = | ||
| mypy sentry_sdk |
There was a problem hiding this comment.
New mypy env missing werkzeug dependency for type checking
Medium Severity
The new [testenv:mypy] environment only installs requirements-linting.txt but is missing werkzeug<2.3.0, which is explicitly included in [testenv:linters] via linters: werkzeug<2.3.0. Because sentry_sdk/integrations/flask.py imports from werkzeug.datastructures import FileStorage, ImmutableMultiDict under TYPE_CHECKING, mypy will attempt to resolve those types. Since werkzeug has no ignore_missing_imports override in pyproject.toml, running tox -e mypy (as instructed in AGENTS.md) will fail with a missing import error that the linters env never surfaces.
sl0thentr0py
left a comment
sl0thentr0py
left a comment
There was a problem hiding this comment.
nice, do we want to keep .cursor or remove it now?
3 participants
Description
Add a tailored AGENTS.md.
Tested with Claude fixing a small issue in the Celery integration. It correctly identified where to make the change, where to add the tests, and how to run the correct test suite.
Issues
Closes #5576
Reminders
tox -e linters.feat:,fix:,ref:,meta:)