8000 misc: reactivate disabled warning by waterlens · Pull Request #2042 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

misc: reactivate disabled warning #2042

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

misc: reactivate disabled warning #2042

wants to merge 1 commit into from

Conversation

waterlens
8000 Copy link
Collaborator

Since moon upgraded, there are no false positive warnings for the extra @bench import. It's time to reactivate the warning now.

@peter-jerry-ye-code-review peter-jerry-ye code review
Copy link
peter-jerry-ye-code-review bot commented Apr 30, 2025 •
edited
Loading
Removal of warning suppression for bench imports

Category
Maintainability
Code Snippet
"warn-list": "-29"
Recommendation
Confirm that all @bench imports are intentional and properly used across the codebase
Reasoning
Since the moon upgrade has resolved false positive warnings for @bench imports, keeping the warning suppression is no longer necessary. Removing it will help catch any actual misuse of bench imports in the future.

@coveralls
Copy link
Collaborator
coveralls commented Apr 30, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 6637

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.378%
Totals Coverage Status
Change from base Build 6636: 0.0%
Covered Lines: 6133
Relevant Lines: 6639
💛 - Coveralls

@waterlens waterlens force-pushed the bench-import branch 4 times, most recently from de8ded9 to 255cb25 Compare May 8, 2025 06:31
@peter-jerry-ye
Copy link
Collaborator

CI is not passing. Do you know why? It seems that it didn't happen as you expected.

@waterlens
Copy link
Collaborator Author
waterlens commented May 9, 2025 •
edited
Loading

CI is not passing. Do you know why? It seems that it didn't happen as you expected.

It's not expected because I wrongly assumed moon check would check the usefulness along with a generated test driver (where @bench is indeed used), but actually it doesn't work like that. The PR shows a false bleeding-CI-passing status when I opened this PR because bleeding-CI ignores the warning.

For now, I don't know what the correct approach to fix it. Some possible directions:

  • Leave the disabled warning.
  • Separate the bench driver and the test driver.
  • Tell people to not use tests with parameters in core, and rewrite the only 1 test requiring it.

cc @Guest0x0

@peter-jerry-ye peter-jerry-ye marked this pull request as draft May 9, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@waterlens @coveralls @peter-jerry-ye
0