-
Notifications
You must be signed in to change notification settings - Fork 50
broaden the publish tag regex to allow digits #2085
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
Conversation
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.
looks good generally, however we might need to adjust a matching regex on the firehose side (we parse the tag to know which package to publish). I'll add a link to this PR
PR HealthBreaking changes ✔️
Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. Coverage ✔️
This check for test coverage is informational (issues shown here will not fail the PR). API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
|
Hmm GitHub seems to also not like this |
Here's the code in firehose where we parse this: https://github.com/dart-lang/ecosystem/blob/main/pkgs/firehose/lib/src/utils.dart#L72 |
I think it didn't like the explicit |
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
Odd - their workflow file validator didn't like the regex? I don't see anything that needs escaping. |
It technically isn't regex its some other custom thing https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#filter-pattern-cheat-sheet |
It seems to me like inside of |
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.
To make sure I understand, we're relying on the fact that we don't match from the start of the tag?
I think that's fine, supporting a trailing numeral in the package name part of the tag looks like a good fix.
Should fix #2084