Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(analyzer): top-level action with existing comments #5273

Merged
merged 6 commits into from
Mar 5, 2025

Conversation

ematipico
Copy link
Member

Summary

This PR fixes an issue where the top-level suppression action wasn't working as expected when modifying a document that contained another comment at the top of the document.

Also, I refactored the trait in a way where the action generated is the same for all languages. The language only provide the string of the comment because it changes depending on the language.

Test Plan

Tested manually

Screen.Recording.2025-03-04.at.15.05.06.mov
Screen.Recording.2025-03-04.at.14.46.59.mov

@github-actions github-actions bot added A-Linter Area: linter A-LSP Area: language server protocol L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages labels Mar 4, 2025
@ematipico ematipico requested review from a team March 4, 2025 14:51
Copy link

codspeed-hq bot commented Mar 4, 2025

CodSpeed Performance Report

Merging #5273 will degrade performances by 6.46%

Comparing fix/top-level-suppression-action (9654d19) with main (4634a8a)

Summary

❌ 1 regressions
✅ 96 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
react.production.min_3378072959512366797.js[cached] 1.9 ms 2 ms -6.46%

ematipico and others added 2 commits March 5, 2025 08:26
Co-authored-by: Arend van Beelen jr. <arendjr@gmail.com>
@ematipico ematipico merged commit c3ceca4 into main Mar 5, 2025
12 checks passed
@ematipico ematipico deleted the fix/top-level-suppression-action branch March 5, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter A-LSP Area: language server protocol L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants