release: v1.3.3 — fix Google Workspace alias domains - #88
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Thank you for creating this pull request and helping make the project better. We will review / merge it when we are online. |
eplus-bot
added
needs-review
PR Summary by QodoRelease v1.3.3: externalize popup changelog and validate release ordering
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
eplus-bot
left a comment
eplus-bot
left a comment
There was a problem hiding this comment.
Approved by @eplus-bot after all pull request checks passed.
CI run: https://github.com/ePlus-DEV/gmail-alias-toolkit/actions/runs/30461184739
|
Approved by @eplus-bot after all pull request checks passed. Approval refresh: #4 CI run: https://github.com/ePlus-DEV/gmail-alias-toolkit/actions/runs/30468210771 |
Code Review by Qodo
1. Changelog test not mirrored
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
eplus-bot
added
app
eplus-bot
left a comment
eplus-bot
left a comment
There was a problem hiding this comment.
Approved by @eplus-bot after all pull request checks passed.
CI run: https://github.com/ePlus-DEV/gmail-alias-toolkit/actions/runs/30463093149
|
Code review by qodo was updated up to the latest commit 6a0b0f2 |
eplus-bot
left a comment
eplus-bot
left a comment
There was a problem hiding this comment.
Approved by @eplus-bot after all pull request checks passed.
CI run: https://github.com/ePlus-DEV/gmail-alias-toolkit/actions/runs/30463477073
eplus-bot
left a comment
eplus-bot
left a comment
There was a problem hiding this comment.
Approved by @eplus-bot after all pull request checks passed.
CI run: https://github.com/ePlus-DEV/gmail-alias-toolkit/actions/runs/30468210771
| import { describe, expect, it } from "vitest"; | ||
| import { CHANGELOG } from "../../entrypoints/popup/data/changelog"; | ||
| import { APP_VERSION } from "../../src/version"; |
There was a problem hiding this comment.
1. Changelog test not mirrored 📘 Rule violation ⚙ Maintainability
tests/popup/changelogData.test.ts does not mirror the source module path/name (entrypoints/popup/data/changelog.ts), violating the repository’s test organization standard and making tests harder to discover and maintain.
Agent Prompt
## Issue description
The new changelog tests are not placed under `tests/` in a structure that mirrors the source module (`entrypoints/popup/data/changelog.ts`), which violates the repo’s test organization standard.
## Issue Context
Current test file is `tests/popup/changelogData.test.ts` while the module under test lives in `entrypoints/popup/data/changelog.ts`.
## Fix Focus Areas
- tests/popup/changelogData.test.ts[1-42]
- entrypoints/popup/data/changelog.ts[1-175]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| function parseVersion(version: string): number[] { | ||
| return version.split(".").map(Number); | ||
| } | ||
|
|
||
| /** Compares two semantic versions from newest to oldest. */ | ||
| function compareVersionsDescending(first: string, second: string): number { | ||
| const firstParts = parseVersion(first); | ||
| const secondParts = parseVersion(second); | ||
|
|
||
| for (let index = 0; index < 3; index += 1) { | ||
| const difference = (secondParts[index] ?? 0) - (firstParts[index] ?? 0); | ||
| if (difference !== 0) return difference; | ||
| } | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
2. Nan semver comparator 🐞 Bug ☼ Reliability
The new changelog test comparator converts version segments with Number(), so versions containing prerelease/build metadata (e.g. "1.4.0-beta.1" or "1.4.0+build.1") produce NaN and make the Array.sort() ordering assertion unreliable (it may not correctly detect misordered versions). This is test-only but can cause false confidence or inconsistent behavior if such versions are introduced later.
Agent Prompt
## Issue description
`parseVersion()` currently does `version.split(".").map(Number)`, which yields `NaN` for semver strings containing prerelease/build metadata (e.g. `1.4.0-beta.1`, `1.4.0+build.1`). This makes `compareVersionsDescending()` potentially return `NaN`, and `Array.sort()` may then treat comparisons as equal, weakening the ordering test.
## Issue Context
This is a test helper, but the repo already considers prerelease/build metadata valid semver in other code, so the changelog test should either:
- explicitly reject such versions (fail fast), or
- compare them correctly.
## Fix Focus Areas
- tests/popup/changelogData.test.ts[6-20]
## Suggested fix
- Parse only the `major.minor.patch` core reliably, e.g.:
- Strip prerelease/build: `const core = version.split(/[+-]/, 1)[0];`
- Validate with a regex: `/^(\d+)\.(\d+)\.(\d+)$/`
- Convert captures via `Number()` and assert they are finite.
- Optionally (if you want full SemVer precedence), add prerelease-aware comparison rather than stripping it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 8b3271e |
|
Thanks for helping make Gmail Alias Toolkit better! |
2 participants
Summary
1.3.2to1.3.3;1.3.3release entry to the changelog displayed inSettings → Changelog;Settings.tsxintoentrypoints/popup/data/changelog.ts;package.json, enforce unique descending versions, and validate release entries.Extension changelog
Fixed
@gmail.com.Structure
Settings.tsxnow only imports the changelog data and renders the UI.Testing
Notes
dev.CHANGELOG.mdis intentionally unchanged.