Draft: Support Unity 6.x #108
Conversation
WalkthroughThe version validation pattern in src/model/image-tag.ts now accepts versions starting with 6000.x and uses full-string anchoring. Corresponding tests in src/model/image-tag.test.ts were added to cover valid 6000.0.0f0 and new invalid cases, ensuring the constructor throws with the specific error message for bad versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Caller
participant IT as ImageTag
participant VP as versionPattern
Dev->>IT: new ImageTag(version)
IT->>VP: Test version against /^(20\d{2}\.\d\.\w{3,4}|6000\.\d\.\w{3,4})$/
alt Match (e.g., 2021.x or 6000.x)
IT-->>Dev: Constructed instance
else No match (e.g., 1999.1.1f1, 6000.1.invalid)
IT-->>Dev: Throw Error("Invalid version \"${version}\".")
end
note over VP: New accepted major: 6000.x<br/>Anchors enforce full-string match
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/model/image-tag.test.ts(1 hunks)src/model/image-tag.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/model/image-tag.test.ts
[error] 11-11: Replace 'accepts·%p·version·format', with ⏎······'accepts·%p·version·format',⏎·····
(prettier/prettier)
[error] 12-12: Insert ··
(prettier/prettier)
[error] 13-13: Replace } with ··},⏎····
(prettier/prettier)
[error] 16-16: English text in string literals is not allowed
(i18n-text/no-en)
🔇 Additional comments (1)
src/model/image-tag.ts (1)
30-30: Verify Unity minor version range. No multi-digit minor versions detected; if Unity allows minors ≥10 (e.g.,2022.10.0f0), update the regex to use\d+for the minor segment:return /^(20\d{2}\.\d+\.\w{3,4}|6000\.\d+\.\w{3,4})$/;
| test.each(['2000.0.0f0', '2011.1.11f1', '6000.0.0f0'])('accepts %p version format', (version) => { | ||
| expect(() => new ImageTag(version)).not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
Fix formatting to comply with Prettier.
The test logic correctly validates the new Unity 6000.x version format alongside existing formats.
Apply this diff to fix the Prettier formatting issues:
- test.each(['2000.0.0f0', '2011.1.11f1', '6000.0.0f0'])('accepts %p version format', (version) => {
+ test.each(['2000.0.0f0', '2011.1.11f1', '6000.0.0f0'])(
+ 'accepts %p version format',
+ (version) => {
- expect(() => new ImageTag(version)).not.toThrow();
- });
+ expect(() => new ImageTag(version)).not.toThrow();
+ },
+ );📝 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.
| test.each(['2000.0.0f0', '2011.1.11f1', '6000.0.0f0'])('accepts %p version format', (version) => { | |
| expect(() => new ImageTag(version)).not.toThrow(); | |
| }); | |
| test.each(['2000.0.0f0', '2011.1.11f1', '6000.0.0f0'])( | |
| 'accepts %p version format', | |
| (version) => { | |
| expect(() => new ImageTag(version)).not.toThrow(); | |
| }, | |
| ); |
🧰 Tools
🪛 ESLint
[error] 11-11: Replace 'accepts·%p·version·format', with ⏎······'accepts·%p·version·format',⏎·····
(prettier/prettier)
[error] 12-12: Insert ··
(prettier/prettier)
[error] 13-13: Replace } with ··},⏎····
(prettier/prettier)
🤖 Prompt for AI Agents
In src/model/image-tag.test.ts around lines 11 to 13, the test block has
Prettier formatting violations; reformat the test.each invocation to comply with
the project's Prettier rules (apply proper indentation, spacing and trailing
commas as configured) — either run Prettier on this file or manually adjust
whitespace so the array, callback, and expect line match the repository style,
then save the file.
| test.each(['1999.1.1f1', '6000.1.invalid'])('rejects %p version format', (version) => { | ||
| expect(() => new ImageTag(version)).toThrow(`Invalid version "${version}".`); | ||
| }); |
There was a problem hiding this comment.
Address i18n linting and formatting.
The test cases correctly validate that invalid version formats are rejected with the appropriate error message.
Apply this diff to fix the formatting and address the i18n linting warning:
- test.each(['1999.1.1f1', '6000.1.invalid'])('rejects %p version format', (version) => {
- expect(() => new ImageTag(version)).toThrow(`Invalid version "${version}".`);
- });
+ test.each(['1999.1.1f1', '6000.1.invalid'])(
+ 'rejects %p version format',
+ (version) => {
+ // eslint-disable-next-line i18n-text/no-en
+ expect(() => new ImageTag(version)).toThrow(`Invalid version "${version}".`);
+ },
+ );Alternatively, if test files should be excluded from i18n linting, update the ESLint configuration to exclude test files from this rule.
📝 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.
| test.each(['1999.1.1f1', '6000.1.invalid'])('rejects %p version format', (version) => { | |
| expect(() => new ImageTag(version)).toThrow(`Invalid version "${version}".`); | |
| }); | |
| test.each(['1999.1.1f1', '6000.1.invalid'])( | |
| 'rejects %p version format', | |
| (version) => { | |
| // eslint-disable-next-line i18n-text/no-en | |
| expect(() => new ImageTag(version)).toThrow(`Invalid version "${version}".`); | |
| }, | |
| ); |
🧰 Tools
🪛 ESLint
[error] 16-16: English text in string literals is not allowed
(i18n-text/no-en)
|
@webbertakken fyi I didn't commit changes to dist, should be done by maintainer for security reasons. |
|
dockerRepoVersion is now 3? |
|
We don't have the rule that this should be done by the maintainers. Please feel free to follow the normal flow. Docker repo can be found here: https://github.com/game-ci/docker/releases |
|
Thanks for the reply, just found out that game-ci/unity-builder handles all the license management. Will close this for now. |
2 participants
Changes
Follow up on #106 fix formatting + add unit test.
Checklist
Summary by CodeRabbit
New Features
Tests