Don't download releases-index.json to resolve major version#560
Conversation
Starting with .NET 5 the minor version is always zero. The earlier releases don't get new versions anymore so we can hardcode the minor version instead of downloading releases-index.json to do the lookup.
akoeplinger
changed the title
|
ping? |
|
@HarithaVattikuti would you mind reviewing this? thanks |
|
ping |
It's insane this repo sees so little activity from the maintainers when just using an agent could close 90% of issues by triage and opening PRs and reviewing open PRs. Can't you escalate internally? |
|
yeah. I'm on vacation for the next few weeks but will see what I can do when I'm back |
|
:O maintainer interaction 5-6 hours ago, closing and re-opening the PR. Any chance to get anything more @HarithaVattikuti ? |
|
Hi @akoeplinger👋, thanks for the contribution! This should fix the Prettier formatting error and ensure the build output is up to date. Please let us know in case of any issues. |
There was a problem hiding this comment.
Pull request overview
This PR simplifies .NET channel resolution for major-only version inputs by avoiding a network call to releases-index.json, based on the assumption that .NET 5+ channels always use a .0 minor and that .NET Core 1–3 no longer change.
Changes:
- Remove the
@actions/http-client-based lookup ofreleases-index.jsonfor resolving major-only version inputs. - Hardcode major-to-channel mappings for .NET Core 1–3 and default to
${major}.0for other numeric majors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@priya-kinthali done, thanks |
|
@priya-kinthali forgot to commit the dist/ file, fixed. there also seems to be a new npm audit error that looks unrelated to the PR |
|
can you please trigger CI again, I think the check-dist should be fixed now |
|
Hi @akoeplinger👋, Thanks for quick updates! It looks like this PR has some merge conflicts. Could you please sync with the latest changes from |
The latest action runs for main fails with npm audit (as reported earlier), will you merge on red after syncing anyways? |
# Conflicts: # dist/setup/index.js # src/installer.ts
akoeplinger
dismissed stale reviews from priya-kinthali and mahabaleshwars
via
15bb176
|
@priya-kinthali merged main, please rerun CI, thanks! |
|
The Basic validation failures are unrelated and are due to #731 |
|
Amazing this got merged, only took 1,5 years. Could get some love as well The PR sat for roughly 18 months despite being small, reviewable, and eventually mergeable. The author pinged multiple times. Once maintainers engaged, the remaining issues were routine formatting/build/review items, which strongly suggests the bottleneck was project triage and ownership, not technical complexity. Timeline |
Description:
Starting with .NET 5 the minor version is always zero. The earlier releases don't get new versions anymore so we can hardcode the minor version instead of downloading releases-index.json to do the lookup.
Check list: