dart-lang / webdev

A CLI for Dart web development.
https://pub.dev/packages/webdev
213 stars 71 forks source link

remove the dep on package:uuid #2415

Closed devoncarew closed 2 months ago

devoncarew commented 2 months ago

This will make the repo a little easier to bring into the sdk repo.


Contribution guidelines:
- See our [contributor guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md) for general expectations for PRs. - Larger or significant changes should be discussed in an issue before creating a PR. - Contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`. - Most changes should add an entry to the changelog and may need to [rev the pubspec package version](https://github.com/dart-lang/sdk/wiki/External-Package-Maintenance#making-a-change). - Changes to packages require [corresponding tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing). Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
devoncarew commented 2 months ago

Happy to add a changelog entry here, though I don't believe this package is published.

kevmoo commented 2 months ago

We should figure out if a UUID is really needed or if we just need a sufficiently random string. Because the code there is WAY more complex than it needs to be. We could just do base64Url with a bunch of bytes and call it a day.

Ditto for the code I swapped out in the SDK

devoncarew commented 2 months ago

Landing as is to facilitate the sdk roll, but if there's any simplification we can do here (wrt less copied code), sgtm.

devoncarew commented 2 months ago

It looks like a few bots are still running (2+ hours?) and have failures. I'm not sure if they're related to this change, but I'll hold off landing for now.

elliette commented 2 months ago

It looks like a few bots are still running (2+ hours?) and have failures. I'm not sure if they're related to this change, but I'll hold off landing for now.

Not related to this change! Feel free to merge. I'm trying to fix the timeouts separately in this PR: https://github.com/dart-lang/webdev/pull/2416 (the --no-sound-null-safety option was removed, so the weak null safety tests were timing out on compilation). I need to figure out why the test timeout wasn't being respected.