feat: add workspace support for packages check licenses#1540
feat: add workspace support for packages check licenses#1540realmeylisdev wants to merge 11 commits intoVeryGoodOpenSource:mainfrom
Conversation
When a pubspec.yaml declares a workspace property, the command now recursively collects dependencies from all workspace members and checks their licenses using the root pubspec.lock. This enables license checking in monorepo projects that use Dart's pub workspace feature. Closes VeryGoodOpenSource#1273
|
@realmeylisdev seems like there are some conflicts that needs to be resolved |
Keep both workspace license support functions and ReporterOutputFormat enum from main.
|
now done. |
|
@marcossevilla could we get this PR reviewed which was de-prioritized last time ? #1444 (comment) |
hi @samitsv, I'll take a look during this week, thanks for pushing this! also mentioning @brianegan as he was having this issue too, a review or test to check if it's fixing your issue is also appreciated 👍 |
|
any update on this @marcossevilla |
|
any update on this @marcossevilla . Would be great to have this merged soon. |
|
hey @samitsv, we're currently trying the branch against internal repos to verify the changes, I'll leave an update once we finish testing |
|
@marcossevilla Functional testing done on my end! PR worked well for a workspace monorepo. Sorry for the delay here -- deadlines. |
| @visibleForTesting | ||
| const pubspecLockBasename = 'pubspec.lock'; | ||
|
|
||
| /// The basename of the pubspec file. | ||
| @visibleForTesting | ||
| const pubspecBasename = 'pubspec.yaml'; |
There was a problem hiding this comment.
if we have a library for pubspecs, maybe it's more natural for these variables to part of it
| /// Attempts to parse a [Pubspec] from a file. | ||
| /// | ||
| /// Returns `null` if the file doesn't exist or cannot be parsed. | ||
| Pubspec? _tryParsePubspec(File pubspecFile) { | ||
| if (!pubspecFile.existsSync()) return null; | ||
| try { | ||
| return Pubspec.fromFile(pubspecFile); | ||
| } on PubspecParseException catch (_) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
same here, maybe it's better if it's part of pubspec library
lib/src/pubspec/pubspec.dart
Outdated
| } on TypeError catch (_) { | ||
| throw const PubspecParseException('Failed to parse YAML content'); | ||
| } on YamlException catch (_) { | ||
| throw const PubspecParseException('Failed to parse YAML content'); | ||
| } |
There was a problem hiding this comment.
| } on TypeError catch (_) { | |
| throw const PubspecParseException('Failed to parse YAML content'); | |
| } on YamlException catch (_) { | |
| throw const PubspecParseException('Failed to parse YAML content'); | |
| } | |
| } on Exception catch (_) { | |
| throw const PubspecParseException('Failed to parse YAML content'); | |
| } |
since it's the same exception being thrown
test/src/pubspec/pubspec_test.dart
Outdated
| import 'package:very_good_cli/src/pubspec/pubspec.dart'; | ||
|
|
||
| void main() { | ||
| group('$Pubspec', () { |
There was a problem hiding this comment.
| group('$Pubspec', () { | |
| group(Pubspec, () { |
test/src/pubspec/pubspec_test.dart
Outdated
| }); | ||
| }); | ||
|
|
||
| group('$PubspecParseException', () { |
There was a problem hiding this comment.
| group('$PubspecParseException', () { | |
| group(PubspecParseException, () { |
test/src/pubspec/pubspec_test.dart
Outdated
| }); | ||
| }); | ||
|
|
||
| group('$PubspecResolution', () { |
There was a problem hiding this comment.
| group('$PubspecResolution', () { | |
| group(PubspecResolution, () { |
Address review feedback on PR VeryGoodOpenSource#1540: - Move `pubspecBasename` constant from licenses.dart to pubspec library - Replace private `_tryParsePubspec` helper with `Pubspec.tryParse` static - Simplify `Pubspec.fromString` by replacing the throwing `as` cast with an `is!` check, removing the `TypeError` catch and lint suppression - Pass class types directly to `group()` in pubspec_test.dart
4 participants
Summary
This PR adds workspace support to the
packages check licensescommand, enabling license checking in monorepo projects that use Dart's pub workspace feature.Reopens #1444 (branch was restored per request).
Closes #1273
Changes
New Files
lib/src/pubspec/pubspec.dart- Pubspec parser for detecting workspace configurationstest/src/pubspec/pubspec_test.dart- 19 tests for the Pubspec parserModified Files
lib/src/commands/packages/commands/check/commands/licenses.dart- Added workspace detection and dependency collectiontest/src/commands/packages/commands/check/commands/licenses_test.dart- Added 3 workspace support testsHow It Works
pubspec.yamlcontains aworkspaceproperty, it's detected as a workspace rootpubspec.yamlfilespackages/*are supported (Dart 3.11+)Test plan