chore(ci): improve clippy annotation reporter comment#2229
Conversation
BenchmarksComparisonCandidateCandidate benchmark detailsBaselineBaseline benchmark details |
ekump
force-pushed
the
ekump/APMSP-3782-reduce-verbosity-of-clippy-annotation-reporter
branch
from
e1c4567 to
81ce3fb
Compare
|
🔄 Datadog auto-retried 1 job - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 808f30e | Docs | Datadog PR Page | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81ce3fbffa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Whether the number of tracked annotations differs between the base and | ||
| /// PR branches. When this is false there is nothing to report. | ||
| pub fn has_changes(&self) -> bool { | ||
| self.base_counts != self.head_counts |
There was a problem hiding this comment.
When a PR removes a tracked allow for a rule from one file or crate and adds the same rule elsewhere, the per-rule totals compare equal here, so main takes the no-change path and deletes/skips the comment. In that scenario the new file/crate breakdown would have non-empty deltas, so the reporter misses the crate/file-level annotation changes this PR is trying to surface; base this predicate on those deltas too, not only the rule totals.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deliberate trade-off to reduce complexity. It won't reflect removing an annotation in a file in one place, and adding one in another place in that file. The diff count is the same.
|
|
||
| // Nothing changed: don't post a comment, and remove any stale one left over | ||
| // from an earlier revision of this PR. | ||
| if !analysis_result.has_changes() { |
There was a problem hiding this comment.
This cleanup only runs after run_analysis returns an AnalysisResult, but run_analysis returns the No Rust files changed error for a PR whose Rust changes have all been removed and the error branch returns before this block. If an earlier revision of the same PR posted a reporter comment and the PR is later updated to touch no Rust files, the stale comment is left behind even though there are no baseline diffs to report; route that no-Rust path through the same delete helper.
Useful? React with 👍 / 👎.
gh-worker-dd-mergequeue-cf854d
Bot
merged commit 57cae97
into
main
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
ekump/APMSP-3782-reduce-verbosity-of-clippy-annotation-reporter
branch
2 participants
What does this PR do?
Motivation
libdatadog guild request
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Temporarily added annotations to this PR to validate comment creation and suppression when no changes.