-
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
Write comments on forks for firehose
#165
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/lib/firehose.dart | 💔 Not covered |
pkgs/firehose/lib/src/github.dart | 💚 12 % ⬆️ 18 % |
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
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.
A few questions:
- I only see one comment on this PR - when using both the health and publish workflows, their output will be combined into one comment? Or is one overwriting the other?
- just confirming that people using this new version of the workflow don't need update their workflow files (ala https://github.com/dart-lang/tools/blob/main/.github/workflows/publish.yaml)
- not being able to update the comment could be problematic. It might be OK for the publish workflow - if we made the normal publish comment super terse - but I could see it being an issue for the health workflow
It took me a while to remember - any changes to the
Yes, as long as they reference
Updating will still work fine, but there was some logic before to delete the comment on workflow failure for the |
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.
Updating will still work fine
Ah, great. I misunderstood - I'd thought this would create new comments on each run.
Fixes #156 .
The behavior does change as comments are never deleted anymore; if we want to keep this behavior the TS in
.github/workflows/post_summaries.yaml
has to be adapted (and we need to store this as an extra parameter passed to the workflow through upload).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.