-
Notifications
You must be signed in to change notification settings - Fork 221
Add custom sdk build options #4301
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
base: master
Are you sure you want to change the base?
Conversation
Markzipan
left a comment
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.
The changes look good!
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
Markzipan
left a comment
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.
LGTM!
Feel free to submit after changelog/version updates
cb30123 to
36cadd8
Compare
|
Thanks for the approval. I did one renaming of the option related to allowing unsafe modules, because I had a read through the webdev issue and it seems like this is more related than I initially though. Since I don't want to conflict here with anything discussed there I opted for the renaming prefixed with "unsafe" and also not publicly documenting this flag. Lmk if the changelog is ok like this, or if I should document the other added flags explicitly. |
36cadd8 to
de73a9a
Compare
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
|
Changelog looks fine! 'Non-user-visible' changes don't need to be included there, so your reasoning's sound. Pulling @bkonyi into this as an FYI; they've been following some efforts to allow these kinds of "flutter can use this lib but I can't" kind of situations. |
| @@ -1,3 +1,7 @@ | |||
| ## 4.4.6 | |||
|
|
|||
| - Add build options to customize the SDK used for compiling to js and wasm. | |||
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.
Is this documented anywhere? How would one use it?
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.
Currently not, but I'd like to.
Where would be a good place to document this? I think this shouldn't be too visible (so not in Readme) since it's quite a special case.
|
The other build options are in the readme, right? It's fine to add an "advanced" section or similar. Just don't want this to get lost in the weeds! |
For #4299
Contribution guidelines:
dart format.Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.