Skip to content

Don't download releases-index.json to resolve major version#560

Merged
HarithaVattikuti merged 9 commits into
actions:mainfrom
akoeplinger:fix-releases
May 11, 2026
Merged

Don't download releases-index.json to resolve major version#560
HarithaVattikuti merged 9 commits into
actions:mainfrom
akoeplinger:fix-releases

Conversation

@akoeplinger
Copy link
Copy Markdown
Contributor

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:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

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 akoeplinger requested a review from a team as a code owner November 7, 2024 13:38
@akoeplinger akoeplinger changed the title Don't download releases-index.json to resolving major version Don't download releases-index.json to resolve major version Nov 7, 2024
@akoeplinger
Copy link
Copy Markdown
Contributor Author

ping?

@akoeplinger
Copy link
Copy Markdown
Contributor Author

@HarithaVattikuti would you mind reviewing this? thanks

@akoeplinger
Copy link
Copy Markdown
Contributor Author

ping

@Frulfump
Copy link
Copy Markdown

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?

@akoeplinger
Copy link
Copy Markdown
Contributor Author

yeah. I'm on vacation for the next few weeks but will see what I can do when I'm back

@Frulfump
Copy link
Copy Markdown

:O maintainer interaction 5-6 hours ago, closing and re-opening the PR. Any chance to get anything more @HarithaVattikuti ?

@priya-kinthali
Copy link
Copy Markdown
Contributor

Hi @akoeplinger👋, thanks for the contribution!
We have noticed that the CI checks are currently failing due to a formatting issue in src/installer.ts. Could you please run the following commands locally and commit the results?

npm run format
npm run build

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.

Copilot AI review requested due to automatic review settings April 16, 2026 09:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of releases-index.json for resolving major-only version inputs.
  • Hardcode major-to-channel mappings for .NET Core 1–3 and default to ${major}.0 for other numeric majors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/installer.ts Outdated
Comment thread src/installer.ts Outdated
Comment thread src/installer.ts Outdated
Comment thread src/installer.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@akoeplinger
Copy link
Copy Markdown
Contributor Author

@priya-kinthali done, thanks

@akoeplinger
Copy link
Copy Markdown
Contributor Author

akoeplinger commented Apr 16, 2026

@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

@akoeplinger
Copy link
Copy Markdown
Contributor Author

can you please trigger CI again, I think the check-dist should be fixed now

mahabaleshwars
mahabaleshwars previously approved these changes Apr 22, 2026
priya-kinthali
priya-kinthali previously approved these changes Apr 22, 2026
@priya-kinthali
Copy link
Copy Markdown
Contributor

Hi @akoeplinger👋, Thanks for quick updates! It looks like this PR has some merge conflicts. Could you please sync with the latest changes from main so we can proceed with merging? Thanks again!

@Frulfump
Copy link
Copy Markdown

Frulfump commented Apr 29, 2026

Hi @akoeplinger👋, Thanks for quick updates! It looks like this PR has some merge conflicts. Could you please sync with the latest changes from main so we can proceed with merging? Thanks again!

The latest action runs for main fails with npm audit (as reported earlier), will you merge on red after syncing anyways?
(And can't you rebase the branch as maintainers?)

# Conflicts:
#	dist/setup/index.js
#	src/installer.ts
@akoeplinger akoeplinger dismissed stale reviews from priya-kinthali and mahabaleshwars via 15bb176 May 7, 2026 11:18
@akoeplinger
Copy link
Copy Markdown
Contributor Author

@priya-kinthali merged main, please rerun CI, thanks!

@akoeplinger
Copy link
Copy Markdown
Contributor Author

The Basic validation failures are unrelated and are due to #731

@HarithaVattikuti HarithaVattikuti merged commit f1970f5 into actions:main May 11, 2026
111 of 114 checks passed
@Frulfump
Copy link
Copy Markdown

Amazing this got merged, only took 1,5 years.

Let's hope
#538
#717

Could get some love as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants