ci: Run with Dart 3.3 and 3.6 in CI pipeline tests #38
Conversation
|
Warning Rate limit exceeded@christerswahn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes introduce multi-version Dart SDK testing in the GitHub Actions CI workflow by running formatting, analysis, and test jobs against both Dart 3.3 and 3.6. Several test files are updated with analyzer directives to suppress warnings related to Dart 3.3 compatibility and unused local variables. In the configuration logic, a return value is updated to use a compile-time constant for the "no value" case. No changes are made to public APIs or exported entities. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Dart Setup Action
participant Job (Format/Analyze/Test)
GitHub Actions->>Job (Format/Analyze/Test): Start job with matrix (Dart 3.3, 3.6)
Job (Format/Analyze/Test)->>Dart Setup Action: Setup Dart SDK ${{ matrix.dart }}
Dart Setup Action-->>Job (Format/Analyze/Test): Dart SDK ready
Job (Format/Analyze/Test)->>Job (Format/Analyze/Test): Run formatting, analysis, or tests
Job (Format/Analyze/Test)-->>GitHub Actions: Report results for each Dart version
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
christerswahn
force-pushed
the
config
branch
2 times, most recently
from
a281fcd to
008e463
Compare
christerswahn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
22-22: Correct indentation forsdk:underwith:YAMLlint flags this line as mis-indented (16 vs. expected 18 spaces). Align it properly:
- sdk: ${{ matrix.dart }} + sdk: ${{ matrix.dart }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 18 but found 16
(indentation)
35-35: Correct indentation forsdk:underwith:Apply the same two-space indent fix here to satisfy YAMLlint.
- sdk: ${{ matrix.dart }} + sdk: ${{ matrix.dart }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 35-35: wrong indentation: expected 18 but found 16
(indentation)
48-48: Correct indentation forsdk:underwith:Repeat the two-space indent adjustment to clear the YAMLlint warning.
- sdk: ${{ matrix.dart }} + sdk: ${{ matrix.dart }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 48-48: wrong indentation: expected 18 but found 16
(indentation)
🛑 Comments failed to post (4)
test/std_out_logger_test.dart (1)
1-1:
⚠️ Potential issueInvalid ignore directive syntax
The comment// ignore required for Dart 3.3is not recognized by the Dart analyzer. To suppress a lint related torequiredparameters under Dart 3.3, use a validignore_for_file:directive—for example:// ignore_for_file: avoid_required_named_parameterstest/prompts/multiple_select_test.dart (1)
1-1:
⚠️ Potential issueInvalid ignore directive syntax
The comment// ignore required for Dart 3.3is not a valid Dart analyzer directive. Replace it with a properignore_for_file:directive to suppress the relevant lint, e.g.:// ignore_for_file: avoid_required_named_parameterstest/prompts/confirm_test.dart (1)
1-1:
⚠️ Potential issueInvalid ignore directive syntax
The comment// ignore required for Dart 3.3will not be honored by the Dart analyzer. To suppress the lint forrequiredparameters under Dart 3.3, use a valid ignore directive:// ignore_for_file: avoid_required_named_parameterstest/prompts/select_test.dart (1)
1-1:
⚠️ Potential issueInvalid ignore directive syntax
The// ignore required for Dart 3.3comment is not a standard analyzer directive and will be ignored. Use the correct syntax to suppress the lint, e.g.:// ignore_for_file: avoid_required_named_parameters
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
22-22: Consider fixing indentation for consistent YAML style.While functionally correct, these lines are flagged by YAMLlint as having inconsistent indentation (16 spaces instead of 18).
- sdk: ${{ matrix.dart }} + sdk: ${{ matrix.dart }}Also applies to: 35-35, 48-48
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 18 but found 16
(indentation)
🛑 Comments failed to post (3)
test/config/configuration_test.dart (3)
1203-1273:
⚠️ Potential issueAdded comprehensive test case for mutually exclusive options with allowed defaults.
This new test group thoroughly verifies the behavior of mutually exclusive options where defaults are allowed, including validation of precedence and error conditions. However, it also depends on the undefined
MutuallyExclusiveModeenum.The test is logically complete but will fail compilation due to the undefined enum. The actual test cases show proper coverage of various scenarios including default value handling.
🧰 Tools
🪛 GitHub Actions: CLI Tools CI
[error] 1208-1208: The named parameter 'mode' isn't defined. Try correcting the name to an existing named parameter's name, or defining a named parameter with the name 'mode'. (undefined_named_parameter)
[error] 1208-1208: Invalid constant value. (invalid_constant)
[error] 1208-1208: Undefined name 'MutuallyExclusiveMode'. Try correcting the name to one that is defined, or defining the name. (undefined_identifier)
1170-1201:
⚠️ Potential issueNew test group added for mutually exclusive mandatory options with default values.
The test validates appropriate error handling when mutually exclusive options are both mandatory and have default values, but it depends on the undefined
MutuallyExclusiveModeenum.The test is well-structured but will fail compilation due to the undefined enum reference. This needs to be fixed before the test can run properly.
🧰 Tools
🪛 GitHub Actions: CLI Tools CI
[error] 1175-1175: The named parameter 'mode' isn't defined. Try correcting the name to an existing named parameter's name, or defining a named parameter with the name 'mode'. (undefined_named_parameter)
[error] 1175-1175: Invalid constant value. (invalid_constant)
[error] 1175-1175: Undefined name 'MutuallyExclusiveMode'. Try correcting the name to one that is defined, or defining the name. (undefined_identifier)
1057-1061: 💡 Verification agent
🧩 Analysis chain
Missing enum definition for MutuallyExclusiveMode.
The test is trying to use a
MutuallyExclusiveMode.mandatoryenum that isn't defined or imported, causing multiple compilation errors.
🏁 Script executed:
#!/bin/bash # Let's check if MutuallyExclusiveMode is defined elsewhere in the codebase echo "Searching for MutuallyExclusiveMode definition or usage" rg "MutuallyExclusiveMode" --type dartLength of output: 406
🏁 Script executed:
#!/bin/bash echo "Searching for enum MutuallyExclusiveMode definition" rg "enum MutuallyExclusiveMode" -n --type dart echo echo "Searching for class MutuallyExclusive definition" rg "class MutuallyExclusive" -n --type dart echo echo "Printing the first 40 lines of test/config/configuration_test.dart to inspect imports" sed -n '1,40p' test/config/configuration_test.dartLength of output: 1889
🏁 Script executed:
#!/bin/bash echo "Displaying the beginning of lib/src/config/option_groups.dart to inspect MutuallyExclusive and related definitions" sed -n '1,200p' lib/src/config/option_groups.dartLength of output: 1507
Update
MutuallyExclusivetests to use named boolean parametersThe
MutuallyExclusiveconstructor doesn’t accept amodeenum; it usesmandatoryandallowDefaultsnamedboolparameters. Update the tests accordingly:• File:
test/config/configuration_test.dart
Lines: ~1057–1061Suggested diff:
-group('Given two options that are mutually exclusive, mandatory,', () { - const group = MutuallyExclusive( - 'mutex-group', - mode: MutuallyExclusiveMode.mandatory, - ); +group('Given two options that are mutually exclusive, mandatory,', () { + const group = MutuallyExclusive( + 'mutex-group', + mandatory: true, + );And for tests using
MutuallyExclusiveMode.allowDefaults, replace:- mode: MutuallyExclusiveMode.allowDefaults, + allowDefaults: true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.group('Given two options that are mutually exclusive, mandatory,', () { const group = MutuallyExclusive( 'mutex-group', mandatory: true, );🧰 Tools
🪛 GitHub Actions: CLI Tools CI
[error] 1060-1060: The named parameter 'mode' isn't defined. Try correcting the name to an existing named parameter's name, or defining a named parameter with the name 'mode'. (undefined_named_parameter)
[error] 1060-1060: Invalid constant value. (invalid_constant)
[error] 1060-1060: Undefined name 'MutuallyExclusiveMode'. Try correcting the name to one that is defined, or defining the name. (undefined_identifier)
This package specifies Dart SDK dependency of 3.3 or greater, but did not test it with Dart 3.3 in the CI pipeline. This PR fixes that.
Summary by CodeRabbit