-
Notifications
You must be signed in to change notification settings - Fork 4
feat!: Moved config library to new config pkg #57
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
The content removed from these files has been moved to the config package.
The content of these files has been copied/moved from the cli_tools package.
📝 WalkthroughWalkthroughThis change extracts the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Tools
participant Config_Package
User->>CLI_Tools: Runs CLI command
CLI_Tools->>Config_Package: Imports and resolves configuration
Config_Package->>Config_Package: Parses args, env, config files
Config_Package-->>CLI_Tools: Returns resolved configuration
CLI_Tools-->>User: Executes command with configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
♻️ Duplicate comments (4)
packages/config/test/better_command_runner/exit_exceptions_test.dart (1)
2-2: Same package-name clash as noted in logging_test.dart
See previous comment; allpackage:config/...imports will need to change if the package is renamed.packages/config/test/config/args_compatibility/parse_performance_test.dart (1)
8-8: Import path update acknowledgedSame remark as in args_test.dart – path change is correct for the new package layout.
packages/config/test/config/args_compatibility/usage_test.dart (1)
7-7: Import path update acknowledgedMatches new standalone
configpackage.packages/config/test/config/configuration_test.dart (1)
4-4: Import path update acknowledgedPoints at the new package; implementation looks fine.
🧹 Nitpick comments (7)
README.md (1)
1-1: Prefer relative link to avoid hard-coding the GitHub domainKeeping the banner’s target as a repository-relative link (
/serverpod/serverpod) avoids an unnecessary protocol/domain hard-coding and still works both on GitHub and when the README is rendered elsewhere (e.g. pkg.dev).-[](https://github.com/serverpod/serverpod) +[](/serverpod/serverpod)packages/config/test/better_command_test.dart (1)
3-4: Consider relying on the barrel export to reduce churnIf
better_command_runner.dartis re-exported byconfig.dart, you can drop the direct import and justimport 'package:config/config.dart';to cut down on import maintenance whenever files move.
Not blocking, just a stylistic hint.packages/config/example/config_file_example.dart (1)
4-5: Minor import tidy-up opportunity.If
better_command_runner.dartis already re-exported bypackage:config/config.dart, you can drop the second import for brevity:-import 'package:config/config.dart'; -import 'package:config/better_command_runner.dart'; +import 'package:config/config.dart';Otherwise, feel free to ignore.
packages/config/example/config_simple_example.dart (1)
1-2: Same import simplification applies here.Consider relying on a single
package:config/config.dartexport if it already exposesBetterCommandRunner.packages/config/README.md (3)
54-55: Fix typo “Tracability” → “Traceability”- - Tracability - the information on an option's value source is retained. +- Traceability – the information on an option's value source is retained.
228-230: Correct typo “specifed” → “specified”- - Non-const values are specifed with a callback using `fromDefault` + - Non-const values are specified with a callback using `fromDefault`
72-74: Tighten wording for clarityThe phrase “with the exception of addCommand(), which you can replace with …” is a bit verbose. Consider something like:
-It achieves complete compatibility with the original package with the exception -of addCommand(), which you can replace with -[`BetterCommandRunner`](lib/src/better_command_runner/better_command_runner.dart). +It is fully compatible except for `addCommand()`, which is covered by +[`BetterCommandRunner`](lib/src/better_command_runner/better_command_runner.dart).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/cli_tools/pubspec.lockis excluded by!**/*.lockpackages/config/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.github/workflows/ci.yml(3 hunks).github/workflows/publish-cli_tools.yaml(1 hunks).github/workflows/publish-config.yaml(1 hunks)README.md(1 hunks)packages/cli_tools/CHANGELOG.md(1 hunks)packages/cli_tools/README.md(3 hunks)packages/cli_tools/example/main.dart(1 hunks)packages/cli_tools/lib/cli_tools.dart(0 hunks)packages/cli_tools/lib/src/documentation_generator/documentation_generator.dart(1 hunks)packages/cli_tools/lib/src/prompts/select.dart(1 hunks)packages/cli_tools/pubspec.yaml(2 hunks)packages/cli_tools/pubspec_overrides.yaml(1 hunks)packages/cli_tools/test/documentation_generator/generate_markdown_test.dart(1 hunks)packages/cli_tools/test/prompts/multiple_select_test.dart(1 hunks)packages/cli_tools/test/prompts/select_test.dart(1 hunks)packages/config/.gitignore(1 hunks)packages/config/CHANGELOG.md(1 hunks)packages/config/LICENSE(1 hunks)packages/config/README.md(1 hunks)packages/config/analysis_options.yaml(1 hunks)packages/config/example/config_file_example.dart(1 hunks)packages/config/example/config_simple_example.dart(1 hunks)packages/config/lib/src/better_command_runner/better_command.dart(1 hunks)packages/config/lib/src/better_command_runner/better_command_runner.dart(1 hunks)packages/config/lib/src/config/configuration.dart(1 hunks)packages/config/lib/src/config/exceptions.dart(1 hunks)packages/config/lib/src/config/options.dart(1 hunks)packages/config/pubspec.yaml(1 hunks)packages/config/test/better_command_runner/analytics_test.dart(1 hunks)packages/config/test/better_command_runner/command_test.dart(1 hunks)packages/config/test/better_command_runner/default_flags_test.dart(1 hunks)packages/config/test/better_command_runner/exit_exceptions_test.dart(1 hunks)packages/config/test/better_command_runner/logging_test.dart(1 hunks)packages/config/test/better_command_runner/parse_log_level_test.dart(1 hunks)packages/config/test/better_command_test.dart(1 hunks)packages/config/test/config/args_compatibility/args_test.dart(1 hunks)packages/config/test/config/args_compatibility/parse_performance_test.dart(1 hunks)packages/config/test/config/args_compatibility/parse_test.dart(1 hunks)packages/config/test/config/args_compatibility/trailing_options_test.dart(1 hunks)packages/config/test/config/args_compatibility/usage_test.dart(1 hunks)packages/config/test/config/config_source_test.dart(1 hunks)packages/config/test/config/configuration_test.dart(1 hunks)packages/config/test/config/configuration_type_test.dart(1 hunks)packages/config/test/config/date_parsing_test.dart(1 hunks)packages/config/test/config/duration_parsing_test.dart(1 hunks)packages/config/test/config/file_options_test.dart(1 hunks)packages/config/test/test_utils/test_utils.dart(1 hunks)
💤 Files with no reviewable changes (1)
- packages/cli_tools/lib/cli_tools.dart
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: the codebase relies on a recent version of `package:args` where `argparser.addflag` accepts the `hid...
Learnt from: christerswahn
PR: serverpod/cli_tools#47
File: lib/src/config/options.dart:552-567
Timestamp: 2025-06-12T14:55:38.006Z
Learning: The codebase relies on a recent version of `package:args` where `ArgParser.addFlag` accepts the `hideNegatedUsage` parameter.
Applied to files:
packages/config/test/config/args_compatibility/trailing_options_test.dartpackages/config/test/better_command_runner/logging_test.dartpackages/config/test/config/configuration_test.dartpackages/config/test/better_command_runner/command_test.dartpackages/config/test/config/args_compatibility/parse_performance_test.dartpackages/config/test/better_command_runner/exit_exceptions_test.dartpackages/config/example/config_simple_example.dartpackages/config/test/better_command_runner/default_flags_test.dartpackages/config/test/config/date_parsing_test.dartpackages/config/test/better_command_runner/parse_log_level_test.dartpackages/config/test/config/args_compatibility/parse_test.dartpackages/cli_tools/test/prompts/multiple_select_test.dartpackages/config/lib/src/better_command_runner/better_command_runner.dartpackages/config/test/config/duration_parsing_test.dartpackages/config/test/config/args_compatibility/args_test.dartpackages/config/test/better_command_test.dartpackages/cli_tools/test/documentation_generator/generate_markdown_test.dartpackages/config/test/better_command_runner/analytics_test.dartpackages/cli_tools/README.mdpackages/config/example/config_file_example.dartpackages/config/test/config/args_compatibility/usage_test.dartpackages/config/pubspec.yamlpackages/config/lib/src/config/options.dartpackages/cli_tools/test/prompts/select_test.dartpackages/cli_tools/pubspec.yamlpackages/config/lib/src/better_command_runner/better_command.dartpackages/cli_tools/example/main.dart
📚 Learning: for this project, avoid review comments that simply repeat compile-time errors the dart analyzer alr...
Learnt from: christerswahn
PR: serverpod/cli_tools#47
File: lib/src/config/options.dart:552-567
Timestamp: 2025-06-12T14:55:38.006Z
Learning: For this project, avoid review comments that simply repeat compile-time errors the Dart analyzer already reports; focus on issues not caught by the analyzer.
Applied to files:
packages/config/.gitignorepackages/config/analysis_options.yaml.github/workflows/ci.ymlpackages/config/example/config_file_example.dartpackages/config/lib/src/config/options.dartpackages/cli_tools/example/main.dart
📚 Learning: assertions should not be used in production code in the serverpod/cli_tools repository, as they are ...
Learnt from: christerswahn
PR: serverpod/cli_tools#46
File: lib/src/config/options.dart:313-316
Timestamp: 2025-05-15T07:45:59.256Z
Learning: Assertions should not be used in production code in the serverpod/cli_tools repository, as they are only active in development mode and not in production builds.
Applied to files:
packages/config/analysis_options.yamlREADME.mdpackages/cli_tools/README.md
📚 Learning: the durationparser constructor in the serverpod/cli_tools repository is const, which means it cannot...
Learnt from: christerswahn
PR: serverpod/cli_tools#46
File: lib/src/config/options.dart:313-316
Timestamp: 2025-05-15T07:45:59.256Z
Learning: The DurationParser constructor in the serverpod/cli_tools repository is const, which means it cannot contain runtime validation code such as throwing exceptions. All validation must be done through documentation or other compile-time approaches.
Applied to files:
packages/config/test/config/date_parsing_test.dartpackages/config/test/config/duration_parsing_test.dart
🪛 LanguageTool
packages/config/README.md
[style] ~72-~72: Consider using “except” or “except for”
Context: ...compatibility with the original package with the exception of addCommand(), which you can replace wit...
(WITH_THE_EXCEPTION_OF)
🪛 markdownlint-cli2 (0.17.2)
packages/config/README.md
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
374-374: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
374-374: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
🔇 Additional comments (42)
packages/config/.gitignore (1)
1-4: Looks good – standard Dart ignore file
No issues detected.packages/config/LICENSE (1)
3-17: Confirm correct copyright statementJust double-check that “2025” is the intended first year; if the code predates 2025 you may want a range such as “2024-2025”.
packages/config/test/config/args_compatibility/args_test.dart (1)
10-10: Import path update looks correct – just ensure theconfigpackage is on the test compile pathThe switch to
package:config/config.dartis expected after the package split. No further action required as long aspackages/config/pubspec.yamldeclaresname: configand the library exportsconfig.dart.packages/config/test/better_command_runner/command_test.dart (1)
4-4: Imports correctly updated to new package nameThe switch from
cli_toolstoconfigaligns with the split-out package. Make surepackages/config/pubspec.yamlexposesbetter_command_runner.dart, and that dependent packages addconfigto theirdev_dependenciesso these tests compile.packages/config/test/config/file_options_test.dart (1)
5-5: Import path looks good
package:config/config.dartresolves to the new package entry-point. Confirm that any IDE-generated imports inside theconfigpackage itself continue to use relative paths rather thanpackage:config/...to avoid cyclic deps when the package is published.packages/config/test/better_command_runner/parse_log_level_test.dart (1)
2-2: LGTM – test now targets the standalone packageNo functional change; the rename is consistent with the new layout.
packages/config/test/config/date_parsing_test.dart (1)
3-3: Import updatedLooks correct; no other changes needed.
packages/cli_tools/test/prompts/select_test.dart (1)
7-8: NewExitExceptionimport is appropriateThe symbol is referenced later in the test (Lines 116-122). Double-check that
cli_toolsdeclares adev_dependencyonconfigso that this test continues to compile once the packages are published independently.packages/config/test/config/args_compatibility/trailing_options_test.dart (1)
7-8: Import path correctly migrated to new packageThe change from
package:cli_tools/config.darttopackage:config/config.dartmatches the package split and should compile once the newconfigpackage is on the test runner’s path. Nothing else to flag.packages/config/test/config/args_compatibility/parse_test.dart (1)
9-10: Import path update is accurateThe updated import reflects the new standalone package. Looks good.
packages/config/test/config/configuration_type_test.dart (1)
3-4: Import migrated successfullyImport now targets
package:config/config.dart; no further issues detected.packages/config/test/config/duration_parsing_test.dart (1)
3-4: Import path aligned with package splitMigration to the new
configpackage is correct. No other concerns.packages/config/lib/src/config/exceptions.dart (1)
1-2: Switched to relative import – OKUsing a relative path (
'options.dart') insidelib/src/configkeeps the file internal-to-package and avoids a public API leak. Verified that both files reside in the same directory, so the import will resolve.packages/cli_tools/test/prompts/multiple_select_test.dart (1)
7-8: Imports updated correctly – no stragglers foundAll occurrences of
better_command_runnerhave been switched to the newpackage:config/better_command_runner.dartpath. I ran a global search for any lingering imports frompackage:cli_toolsand found none.packages/config/test/config/config_source_test.dart (1)
3-3: Import relocation LGTMThe test now pulls
config.dartfrom the new standalone package. No further action required..github/workflows/publish-cli_tools.yaml (1)
1-1: Workflow rename acknowledgedThe updated workflow name better reflects the package being published. 👍
packages/config/lib/src/config/configuration.dart (1)
18-18: Docs updated to new package ‒ looks goodThe example now points at
package:config/config.dart, keeping the guide in sync with the refactor.packages/cli_tools/CHANGELOG.md (1)
3-5: Entry reads clearly – no concerns.The new 0.7.0-beta.2 bullet is consistent with the existing style and communicates the breaking refactor unambiguously.
packages/config/lib/src/config/options.dart (1)
3-3: Good switch to a relative import.Using
exceptions.dartkeeps the library’s internal imports private and avoids an unnecessary public package dependency.packages/config/CHANGELOG.md (1)
1-5: Changelog entry looks fine.The initial version note succinctly describes the package extraction and keeps versioning in sync with
cli_tools.packages/config/test/better_command_runner/analytics_test.dart (2)
4-4: LGTM - Correct package import updateThe import has been properly updated to reference the new standalone
configpackage instead ofcli_tools.
7-7: Good practice using explicit importUsing the
showclause to explicitly import onlyflushEventQueueimproves code clarity and reduces namespace pollution.packages/cli_tools/pubspec_overrides.yaml (1)
1-3: LGTM - Proper local dependency overrideThis correctly overrides the
configdependency to use the local package during development, which is the standard approach for monorepo package dependencies.packages/config/analysis_options.yaml (1)
1-5: LGTM - Appropriate analysis configurationThe linting configuration is consistent with project standards using
serverpod_lints/cli.yaml, and theunnecessary_final: falsesetting is a reasonable choice for code clarity.packages/config/test/test_utils/test_utils.dart (1)
1-5: LGTM - Standard async testing utilityThis is a well-documented utility function that follows the standard pattern for flushing the event queue in async tests. The
Future.delayed(Duration.zero)approach is the correct way to allow pending async events to complete.packages/cli_tools/test/documentation_generator/generate_markdown_test.dart (1)
5-6: LGTM - Correct import updates for package separationThe imports have been properly updated to reference the extracted
configpackage. Using explicitshowclauses forConfigurationandOptionDefinitionimproves code clarity.packages/cli_tools/lib/src/documentation_generator/documentation_generator.dart (1)
1-1: LGTM! Clean import update.The import path correctly references the new
configpackage and uses selective importing to only bring inBetterCommandRunner. This aligns perfectly with the package extraction objective.packages/cli_tools/pubspec.yaml (2)
2-2: Version bump is appropriate for breaking change.The version increment from 0.7.0-beta.1 to 0.7.0-beta.2 correctly reflects the breaking nature of extracting the config library.
20-20: Versions are aligned across packagesThe
configdependency (^0.7.0-beta.2) in packages/cli_tools/pubspec.yaml matches theversion: 0.7.0-beta.2in packages/config/pubspec.yaml. No changes needed.packages/config/lib/src/better_command_runner/better_command.dart (1)
6-8: LGTM! Import paths correctly updated for new package structure.The imports properly reference the new
configpackage structure with appropriate selective importing offormatConfigError. These self-referential imports within the config package are correct..github/workflows/ci.yml (3)
17-17: LGTM! Comprehensive CI coverage for both packages.Adding
configto the package matrix ensures bothcli_toolsand the newconfigpackage receive proper CI validation across formatting, analysis, and testing.
33-33: Consistent matrix strategy across all jobs.The dart_analyze job correctly includes both packages in the matrix, maintaining consistency with the formatting job.
51-51: Testing coverage extended to both packages.The dart_test job appropriately covers both packages, ensuring the extracted config functionality is properly validated.
packages/cli_tools/lib/src/prompts/select.dart (1)
4-4: LGTM! Correct import update for ExitException.The import properly references
ExitExceptionfrom the newconfigpackage with selective importing, maintaining the quit functionality for interactive prompts.packages/config/lib/src/better_command_runner/better_command_runner.dart (1)
6-8: LGTM! Import updates correctly reflect the package extraction.The import changes properly reference the new
configpackage instead ofcli_tools. The selective import offormatConfigErroris appropriate and maintains the existing functionality..github/workflows/publish-config.yaml (1)
1-16: LGTM! Well-structured publishing workflow for the new config package.The workflow correctly:
- Uses semantic versioning tag pattern with config prefix
- Leverages the official dart-lang/setup-dart reusable workflow
- Sets appropriate permissions and working directory
- Follows GitHub Actions best practices
packages/config/pubspec.yaml (2)
1-30: LGTM! Well-configured package manifest for the extracted config package.The pubspec.yaml properly defines the new config package with:
- Appropriate version (beta release for initial extraction)
- Relevant dependencies and topics
- Modern Dart SDK constraints
- Correct repository references
6-6: Documentation URL VerifiedThe README file is present at
packages/config/README.md, so thedocumentationURL inpubspec.yamlis correct and requires no changes.packages/cli_tools/README.md (2)
10-10: LGTM! Clear direction to users about the config library extraction.The README correctly informs users that the config library has been moved to its own package with a helpful link to pub.dev.
1-1: Nice improvement making the banner clickable.The banner is now a clickable link to the Serverpod repository, improving user experience.
packages/cli_tools/example/main.dart (1)
6-12: LGTM! Appropriate simplification removing config dependencies.The example correctly removes the complex command runner setup and focuses on demonstrating the core
StdOutLoggerfunctionality. The manual flag parsing is appropriate for this simplified example.packages/config/README.md (1)
192-201: Verify intra-repo links render on pub.devRelative links such as
[OptionDefinition](lib/src/config/configuration.dart)
work on GitHub but are stripped on pub.dev. Consider linking to API
documentation instead, e.g.https://pub.dev/documentation/config/latest/config/OptionDefinition-class.html,
or prefixing withpackage:so they resolve everywhere.
packages/config/test/better_command_runner/default_flags_test.dart
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/config/README.md (3)
52-56: Misspelling: “Tracability” → “Traceability”Minor typo that can undermine professionalism.
[Suggest fix]-- Tracability - the information on an option's value source is retained. +- Traceability – the information on an option's value source is retained.
223-230: Typo in “specifed”Small spelling error in the bullet about default values.
[Suggest fix]- - Non-const values are specifed with a callback using `fromDefault` + - Non-const values are specified with a callback using `fromDefault`
21-24: Broken link on pub.dev forCONTRIBUTING.md
/CONTRIBUTING.mdresolves on GitHub but becomes an absolute path on pub.dev, resulting in a 404. Use a relative or full GitHub URL.-See also [CONTRIBUTING](/CONTRIBUTING.md). +See also [CONTRIBUTING](https://github.com/serverpod/cli_tools/blob/main/CONTRIBUTING.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/config/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: christerswahn
PR: serverpod/cli_tools#57
File: packages/config/test/better_command_runner/logging_test.dart:2-2
Timestamp: 2025-08-07T07:56:11.913Z
Learning: The maintainer christerswahn owns the `config` package name on pub.dev, so using `config` as the package name in this codebase is valid and will not cause publication conflicts.
Learnt from: christerswahn
PR: serverpod/cli_tools#57
File: packages/config/test/better_command_runner/default_flags_test.dart:1-1
Timestamp: 2025-08-07T07:55:17.221Z
Learning: In the `config` package, `better_command_runner.dart` is intentionally kept as a separate import (`package:config/better_command_runner.dart`) rather than being re-exported through the main `packages/config/lib/config.dart` barrel file. This separation is by design according to the package maintainer christerswahn.
📚 Learning: in the `config` package, `better_command_runner.dart` is intentionally kept as a separate import (`p...
Learnt from: christerswahn
PR: serverpod/cli_tools#57
File: packages/config/test/better_command_runner/default_flags_test.dart:1-1
Timestamp: 2025-08-07T07:55:17.221Z
Learning: In the `config` package, `better_command_runner.dart` is intentionally kept as a separate import (`package:config/better_command_runner.dart`) rather than being re-exported through the main `packages/config/lib/config.dart` barrel file. This separation is by design according to the package maintainer christerswahn.
Applied to files:
packages/config/README.md
📚 Learning: the maintainer christerswahn owns the `config` package name on pub.dev, so using `config` as the pac...
Learnt from: christerswahn
PR: serverpod/cli_tools#57
File: packages/config/test/better_command_runner/logging_test.dart:2-2
Timestamp: 2025-08-07T07:56:11.913Z
Learning: The maintainer christerswahn owns the `config` package name on pub.dev, so using `config` as the package name in this codebase is valid and will not cause publication conflicts.
Applied to files:
packages/config/README.md
🪛 LanguageTool
packages/config/README.md
[style] ~72-~72: Consider using “except” or “except for”
Context: ...compatibility with the original package with the exception of addCommand(), which you can replace wit...
(WITH_THE_EXCEPTION_OF)
🪛 markdownlint-cli2 (0.17.2)
packages/config/README.md
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
371-371: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
374-374: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
374-374: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
SandPod
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.
In packages/config/lib/config.dart it is a shame that we need to re-export UsageException. Is it really required by the package?
For some reason I can't comment in the file since it was only subject to a move.
Otherwise changes look good, just had some minor questions.
packages/cli_tools/lib/src/documentation_generator/documentation_generator.dart
Outdated
Show resolved
Hide resolved
Yes, that is part of the contract, it is thrown with error and usage information upon user configuration errors. This is compatible with the args package - although that is inconsistent in how it throws it; we do it in a consistent manner. The idea is that when we replace the underlying ArgParser, we define this exception ourselves. User code will not notice, the reexport will be replaced by our own export. |
SandPod
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.
🚀
Moves the
configlibrary from thecli_toolspackage into the newconfigpackage.configwill be independently published on pub.dev.Summary by CodeRabbit
New Features
configpackage for advanced command-line and configuration file parsing, featuring typed options, environment variable support, and improved error handling.configpackage.Refactor
configlibrary fromcli_toolsinto its own package.cli_toolsandconfigpackages to reflect the separation.Documentation
cli_toolsdocumentation to reference the newconfigpackage.Chores
configpackage.configpackage.Tests
configpackage.