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

ci: add the check-ignore flag in dart coverage #107

Merged
merged 6 commits into from
Apr 20, 2023

Conversation

kmartins
Copy link
Contributor

@kmartins kmartins commented Mar 6, 2023

Description

Add the flag --check-ignore in Dart Coverage for comments below to work:

// coverage:ignore-line to ignore one line.
// coverage:ignore-start and // coverage:ignore-end to ignore the range of lines inclusive.
// coverage:ignore-file to ignore the whole file.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@@ -77,7 +77,7 @@ jobs:
- name: 🧪 Run Tests
run: |
dart pub global activate coverage 1.2.0
dart test -j ${{inputs.concurrency}} --coverage=coverage --platform=${{inputs.platform}} && dart pub global run coverage:format_coverage --lcov --in=coverage --out=coverage/lcov.info --packages=.dart_tool/package_config.json --report-on=${{inputs.report_on}}
dart test -j ${{inputs.concurrency}} --coverage=coverage --platform=${{inputs.platform}} && dart pub global run coverage:format_coverage --lcov --check-ignore --in=coverage --out=coverage/lcov.info --packages=.dart_tool/package_config.json --report-on=${{inputs.report_on}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add this under an option as this could be considered breaking for anyone using this in the next version

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I added a new input for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolfenrain I think we should go forward with the breaking change.

I think it is more sensible to have it enabled by default. Since if you are using comments to ignore particular sections of the code you're most likely doing it purposely already.

@alestiago
Copy link
Contributor

alestiago commented Mar 16, 2023

This might be related to VeryGoodOpenSource/very_good_coverage#200.

renancaraujo
renancaraujo previously approved these changes Mar 16, 2023
.github/workflows/dart_package.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@renancaraujo renancaraujo self-requested a review March 16, 2023 13:23
Copy link
Contributor

@alestiago alestiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the parameter type should be boolean and true by default.

renancaraujo
renancaraujo previously approved these changes Mar 21, 2023
README.md Outdated Show resolved Hide resolved
alestiago
alestiago previously approved these changes Mar 28, 2023
wolfenrain
wolfenrain previously approved these changes Mar 29, 2023
alestiago
alestiago previously approved these changes Apr 5, 2023
renancaraujo
renancaraujo previously approved these changes Apr 18, 2023
README.md Outdated Show resolved Hide resolved
@renancaraujo renancaraujo dismissed stale reviews from alestiago and themself via 0fc6e0f April 20, 2023 17:41
renancaraujo and others added 2 commits April 20, 2023 18:41
Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>
Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>
@renancaraujo renancaraujo merged commit 6bd9182 into VeryGoodOpenSource:main Apr 20, 2023
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