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

Remove coverage import altogether #1160

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Remove coverage import altogether #1160

merged 2 commits into from
Dec 9, 2024

Conversation

lynzrand
Copy link
Collaborator

Remove import of coverage from all modules.

Should be added after moonbitlang/moon#415 is merged, so coverage imports are added automatically.

Copy link

The git diff output provided shows changes in multiple moon.pkg.json files, mainly removing references to "moonbitlang/core/coverage" from the "import" section. Here are some observations and suggestions based on the changes:

  1. Consistency in Package Dependencies:

    • The removal of "moonbitlang/core/coverage" across multiple files suggests that this dependency is no longer needed or is being phased out. This change should be made consistently across all relevant files to ensure that the project builds correctly and does not rely on unused dependencies.
  2. Review Test Dependencies:

    • The "test-import" sections remain unchanged, which is good because it indicates that the testing functionality is not affected by the removal of the coverage dependency. However, it's a good practice to review these sections periodically to ensure that all test dependencies are necessary.
  3. Documentation and Communication:

    • Since this change affects multiple files and is likely part of a broader update, it's important to document the reason for the change (e.g., phasing out coverage or replacing it with a different tool) and communicate this to the team. This helps ensure that everyone is aware of the changes and their implications.

In summary, the changes appear to be focused on removing an unused or deprecated dependency. This is a step towards a cleaner and more efficient project setup. However, it's crucial to ensure that these changes are made consistently across all relevant files and that the team is aware of and understands the rationale behind these changes.

@bobzhang
Copy link
Contributor

bobzhang commented Dec 8, 2024

@lynzrand I tested it on #1306, it does work, can you revive and finish this PR?

@lynzrand lynzrand force-pushed the rynco/remove-coverage-dep branch from 7ece93c to 3751393 Compare December 9, 2024 03:56
@lynzrand lynzrand marked this pull request as ready for review December 9, 2024 03:56
@lynzrand lynzrand requested a review from Young-Flash December 9, 2024 03:56
@coveralls
Copy link
Collaborator

coveralls commented Dec 9, 2024

Pull Request Test Coverage Report for Build 4157

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 80.824%

Totals Coverage Status
Change from base Build 4155: 0.0%
Covered Lines: 4493
Relevant Lines: 5559

💛 - Coveralls

@lynzrand lynzrand merged commit a954f22 into main Dec 9, 2024
13 checks passed
@lynzrand lynzrand deleted the rynco/remove-coverage-dep branch December 9, 2024 07:04
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.

4 participants