Ignore pre-release parts when comparing GHES versions - #2972
Conversation
Sorry, something went wrong.
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 parseGhesVersion function with satisfiesGHESVersion for cleaner version range checking
- Use semver.coerce to 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 |
Sorry, something went wrong.
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.
Sorry, something went wrong.
mbg
left a comment
There was a problem hiding this comment.
Thanks! This looks good now.
Sorry, something went wrong.
| */ | ||
| export function satisfiesGHESVersion( | ||
| githubVersion: GitHubVersion, | ||
| ghesVersion: string, |
There was a problem hiding this comment.
Good idea!
Sorry, something went wrong.
|
There's a merge conflict with lib/upload-lib.js.map which I think you can resolve by merging / rebasing and then running npm run build again |
Sorry, something went wrong.
|
Thanks for the reviews! I've rebased and squashed all three commits into one to make this easier to cherry-pick for the backport. |
Sorry, something went wrong.
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