-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add api tool call for testing #153
Conversation
PR HealthPackage publish validation ✔️Details
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. License Headers ✔️Details
All source files should start with a license header. Changelog Entry ✔️Details
Changes to files need to be accounted for in their respective changelogs. Coverage
|
File | Coverage |
---|---|
pkgs/firehose/bin/health.dart | 💔 Not covered |
pkgs/firehose/lib/src/health/health.dart | 💔 Not covered |
pkgs/firehose/lib/src/repo.dart | 💚 87 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check
Breaking changes ⚠️
Details
Package | Change | Current Version | New Version | Needed Version | Looking good? |
---|---|---|---|---|---|
firehose | Breaking | 0.3.30 | 0.3.31 | 0.4.0 | |
dart_flutter_team_lints | None | 2.0.0 | 2.0.0 | 2.0.0 | ✔️ |
This check can be disabled by tagging the PR with skip-breaking-check
86a9189
to
ada4234
Compare
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
@@ -1,3 +1,7 @@ | |||
## 0.3.31 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fyi there are a few PRs underway rev'ing firehose -
@@ -92,6 +92,9 @@ jobs: | |||
run: dart pub global activate firehose | |||
if: ${{ !inputs.local_debug }} | |||
|
|||
- name: Install api_tool | |||
run: dart pub global activate --source git https://github.com/bmw-tech/dart_apitool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it that this tool isn't published to pub?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is - the version I want is just not published yet.
); | ||
print('Look for changes in $currentPath with base $basePackage'); | ||
var runApiTool = Process.runSync( | ||
'dart-apitool', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps instead 'dart, ['pub', 'global', 'run', 'dart-apitool', ...
? It's the same as what you have, just doesn't rely on the pub tools dir being installed into the PATH ...
'diff', | ||
...['--old', basePackage], | ||
...['--new', '.'], | ||
...['--report-format', 'json'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this run finds breaking changes, should we re-run this report in another format and dump the results to stdout?
This reverts commit ada4234.
Check for breaking changes.
Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.