Delete directories between workspace package and workspace root #4446
Conversation
lib/src/entrypoint.dart
Outdated
| deleteEntry(p.join(dir, 'pubspec.lock')); | ||
| deleteEntry(p.join(dir, '.dart_tool', 'package_config.json')); |
There was a problem hiding this comment.
Should we write a warning message or something to logs.
Maybe something that includes a dart.dev/go/-link, so that we can explain why we deleted these folders.
If someone has workspace packages nested inside non-workspace packages inside a root, that could cause this, right? or probably it'll cause an error when trying to resolve the workspaces packages?
There was a problem hiding this comment.
Should we write a warning message or something to logs.
I'm not completely against this, but I think it might confuse more than it benefits...
Maybe something that includes a dart.dev/go/-link,
I think that would be slightly overdoing it... But maybe we can fit a paragraph in somewhere on the workspaces documentation...
so that we can explain why we deleted these folders.
We are deleting files, not folders.
If someone has workspace packages nested inside non-workspace packages inside a root,
We should probably detect if someone attempts this (by detecting a pubspec.yaml), and give an error message.
That scenario is not supported.
There was a problem hiding this comment.
Ok - now displaying a warning and a doc reference.
Also added an error check if there are stray pubspec.yaml files between a package and the root.
lib/src/entrypoint.dart
Outdated
| } | ||
| } | ||
| if (deletedAny) { | ||
| log.warning( | ||
| 'See https://dart.dev/go/workspaces-no-inbetween-packages for details.', |
There was a problem hiding this comment.
| 'See https://dart.dev/go/workspaces-no-inbetween-packages for details.', | |
| 'See https://dart.dev/go/workspace-packages-in-other-package for details.', |
I'm not sure this is better, just an idea.
lib/src/package.dart
Outdated
| } | ||
|
|
||
| // Check for pubspec.yaml files between the root and any workspace package. | ||
| final visited = <String>{}; |
There was a problem hiding this comment.
Just an idea, feel free to ignore.
| final visited = <String>{}; | |
| final visited = <String>{root.dir}; |
Then you won't need p.equals(dir, root.dir), but then again, maybe this is just too smart. 🤣
Fixes #4445
Also makes resolution error if there are any pubspec.yaml's in such directories.