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

Suggestion: remove code-suggester, print diff instead #2312

Closed
wants to merge 1 commit into from

Conversation

danielwe
Copy link
Contributor

In addition to creating rather noisy PR threads, code-suggester is fundamentally broken as evidenced by all the nonsense suggestions in #1852. Google does not intend to fix this as explained here: googleapis/code-suggester#505 (comment).

So I thought it might be better to skip the suggestions and simply let the CI action pass/fail depending on whether the PR is compliant. Runic's suggested diff can then be found in the CI logs.

Copy link

codecov bot commented Feb 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.46%. Comparing base (037dfed) to head (628aa0e).
Report is 354 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2312      +/-   ##
==========================================
+ Coverage   67.50%   75.46%   +7.96%     
==========================================
  Files          31       56      +25     
  Lines       12668    16738    +4070     
==========================================
+ Hits         8552    12632    +4080     
+ Misses       4116     4106      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielwe
Copy link
Contributor Author

To bolster the case, #2301 is also full of nonsense suggestions such as #2301 (comment)

@vchuravy
Copy link
Member

Thanks, take a look at #2313

@vchuravy vchuravy closed this Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants