Ignore pre-release parts when comparing GHES versions#2972
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the GHES version comparison logic to ignore pre-release version parts when determining compatibility. The change replaces the parseGhesVersion function with a new satisfiesGHESVersion function that uses semver.coerce and explicitly strips pre-release components before performing version comparisons.
- Replace
parseGhesVersionfunction withsatisfiesGHESVersionfor cleaner version range checking - Use
semver.coerceto handle non-standard version formats and strip pre-release components - Update all GHES version comparisons to use the new function with range syntax
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/util.ts | Replaces parseGhesVersion with satisfiesGHESVersion function that strips pre-release parts |
| src/upload-lib.ts | Updates version comparisons to use new function and removes unused semver import |
| lib/util.js | Generated JavaScript for the util.ts changes |
| lib/upload-lib.js | Generated JavaScript for the upload-lib.ts changes |
mbg
left a comment
There was a problem hiding this comment.
Thanks for putting this together! This probably makes sense, since I can't imagine that we'd want to check for a particular pre-release version. Dropping the pre-release for the comparison makes sense to me.
One suggestion about the potential footgun that Copilot highlighted as well.
| */ | ||
| export function satisfiesGHESVersion( | ||
| githubVersion: GitHubVersion, | ||
| ghesVersion: string, |
|
There's a merge conflict with |
e080457 to
e30db30
Compare
|
Thanks for the reviews! I've rebased and squashed all three commits into one to make this easier to cherry-pick for the backport. |
This will change our GHES version comparison to always compare without taking into consideration the pre-release part of the version, and dropping any unrecognized semver parts.
Merge / deployment checklist