Repository navigation
Improve firehose publish validation log output - #468
Conversation
When the publishing PR comment can't be written, the release links were not available anywhere in the workflow log, and the log was dominated by repeated "Found package at ..." lines. - Print the links to create GitHub releases for packages that are ready to publish, along with any validation errors, as the final output of `Firehose.validate`, outside of any group so they are easy to find. - Search the file system for packages at most once per `Repository`. `isSinglePackageRepo` previously re-ran package discovery on every call, and it was called via `calculateRepoTag` and `calculateReleaseUri` for each package. - Group the log output into collapsible sections for package discovery, the validation of each package, and preparing the PR comment. - Extract the per-package validation into `_verifyPackage`. - Wait for subprocess output to be written before `runCommand` returns, so it isn't interleaved with later output or written outside its log group.
|
@kevmoo pointed out theres a different place you can find these links, but the noise cleanup is still worthwhile IMO, and it doesn't hurt to also print the link at the end of the log output. |
There was a problem hiding this comment.
Code Review
This pull request improves the GitHub Actions logging for the publish.yaml workflow by grouping log outputs into collapsible sections, caching package discovery to search the file system at most once per Repository, and printing release links at the end of the validation log. Additionally, it refactors package verification and ensures command output streams are fully written before returning. Feedback suggests concurrently awaiting process.exitCode, stdoutDone, and stderrDone using Future.wait in runCommand to handle potential stream errors more robustly.
| final code = await process.exitCode; | ||
| // Wait for all output to be written so that it isn't interleaved with any | ||
| // output from the caller after this returns. | ||
| await Future.wait([stdoutDone, stderrDone]); |
There was a problem hiding this comment.
Concurrently await process.exitCode, stdoutDone, and stderrDone using Future.wait. This is more robust because if any of the streams encounter an error (e.g., decoding issues) while the process is still running, the error will be caught and propagated immediately rather than being unhandled or delayed.
| final code = await process.exitCode; | |
| // Wait for all output to be written so that it isn't interleaved with any | |
| // output from the caller after this returns. | |
| await Future.wait([stdoutDone, stderrDone]); | |
| final results = await Future.wait([ | |
| process.exitCode, | |
| stdoutDone, | |
| stderrDone, | |
| ]); | |
| final code = results[0] as int; |
|
|
||
| - Print the links to create GitHub releases at the end of the `publish.yaml` | ||
| validation log so they are available when the PR comment can't be written. | ||
| - Group the `publish.yaml` log output into collapsible sections for package |
PR HealthCoverage ✔️
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 License Headers ✔️
All source files should start with a license header. This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with Unused Dependencies ✔️
For details on how to fix these, see dependency_validator. This check can be disabled by tagging the PR with
API leaks
|
| Package | Leaked API symbol | Leaking sources |
|---|---|---|
| firehose | Pub | firehose/firehose.dart::Firehose::verify::pub |
This check can be disabled by tagging the PR with skip-leaking-check.
Changelog Entry ✔️
| Package | Changed Files |
|---|
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check (for example, for non-user-visible changes when the package version is already at -wip).
kevmoo
left a comment
There was a problem hiding this comment.
@natebosch – posted unverified silly then deleted it!
? |
never mind 🙃 |
…, web Revisions updated by `dart tools/rev_sdk_deps.dart`. core (https://github.com/dart-lang/core/compare/4d80610..72e71ca): 72e71ca0 Mon Oct 19 21:57:11 2026 +0200 Jonas Finnemann Jensen Optimize `HashSink` and `_MD5Sink.updateHash` (RFC impl) (dart-lang/core#1013) dartdoc (https://github.com/dart-lang/dartdoc/compare/e1859cf..c0e19cf): c0e19cf6 Mon Oct 19 11:41:42 2026 -0700 Konstantin Scheglov Import analyzer ast with explicit show. (dart-lang/dartdoc#4300) ecosystem (https://github.com/dart-lang/ecosystem/compare/622ef61..0ab0cfd): 0ab0cfd Mon Oct 19 15:44:26 2026 +0200 Moritz fix(firehose): bump dependency_validator to support analyzer 14 (dart-lang/ecosystem#453) 44a0768 Wed Oct 14 09:37:29 2026 -0700 Nate Bosch Improve firehose publish validation log output (dart-lang/ecosystem#468) http (https://github.com/dart-lang/http/compare/7d7aae2..a8a24a0): a8a24a0 Mon Oct 19 13:31:34 2026 -0700 Kevin Moore docs(web_socket): remove non-user-visible changelog entry (dart-lang/http#2023) 6f6ac5f Mon Oct 19 16:29:43 2026 -0400 Mouad Debbar [http_multi_server] Retry `loopback(0)` when IPv6 port is already bound (dart-lang/http#2025) 6e91ce3 Tue Oct 13 16:25:39 2026 -0700 Kevin Moore Sync root README.md package table from pubspec.yaml files (dart-lang/http#2024) native (https://github.com/dart-lang/native/compare/cc7e4b0..3cbe319): 3cbe319e3 Tue Oct 13 15:47:18 2026 +0200 Daco Harkes [hooks_runner] Fail eagerly when file locking is not supported on file system (dart-lang/native#3679) shelf (https://github.com/dart-lang/shelf/compare/b2cb503..b66665b): b66665b Fri Oct 16 15:12:05 2026 -0700 Kevin Moore chore: add health workflow and prepare shelf 1.4.3, shelf_router 1.1.5, and shelf_static 1.2.0 (dart-lang/shelf#546) test (https://github.com/dart-lang/test/compare/9bf69a8..d1afeb1): d1afeb14 Fri Oct 16 20:41:51 2026 -0400 Mouad Debbar Confirmation that `tearDown` is called on timeout (dart-lang/test#2798) 3f6c2f14 Thu Oct 15 18:21:35 2026 -0700 Jacob MacDonald Enable asserts in the cli compiler (dart-lang/test#2795) tools (https://github.com/dart-lang/tools/compare/c1471db..f6c7a2d): f6c7a2d62 Mon Oct 19 07:59:24 2026 -0700 Kevin Moore [pubspec_parse] Preserve input maps and YamlMap source spans (dart-lang/tools#2653) f1a55d953 Mon Oct 19 07:53:51 2026 -0700 Kevin Moore [yaml] Fix tag URI decoding, numeric tag RangeErrors, and dead checks (dart-lang/tools#2630) 340ded284 Mon Oct 19 07:43:31 2026 -0700 Kevin Moore [unified_analytics] Prepare for 8.2.0 release (dart-lang/tools#2689) a73703543 Fri Oct 16 11:55:03 2026 -0700 Kevin Moore [unified_analytics] Bump version to 8.2.0-wip (dart-lang/tools#2688) web (https://github.com/dart-lang/web/compare/63e6662..4e60e61): 4e60e61 Mon Oct 19 10:02:22 2026 -0700 dependabot[bot] Bump source-map-js from 1.2.1 to 1.2.2 in /web_generator (dart-lang/web#591) 7d63637 Mon Oct 19 09:50:26 2026 -0700 Kevin Moore Add an api.txt golden for package:web (dart-lang/web#590) [email protected] Change-Id: Ibe6ac8a8246a27caa8a4846ea1acc78c801ee2ae Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/566781 Commit-Queue: Nate Bosch <[email protected]> Reviewed-by: Brian Quinlan <[email protected]>
When the publishing PR comment can't be written, the release links were
not available anywhere in the workflow log, and the log was dominated by
repeated "Found package at ..." lines.
to publish, along with any validation errors, as the final output of
Firehose.validate, outside of any group so they are easy to find.Repository.isSinglePackageRepopreviously re-ran package discovery on everycall, and it was called via
calculateRepoTagandcalculateReleaseUrifor each package.the validation of each package, and preparing the PR comment.
_verifyPackage.runCommandreturns,so it isn't interleaved with later output or written outside its log
group.