Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dotnet lts fix #1003

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

prathameshzarkar9
Copy link
Contributor

Issue 1000
Feature Name:

  • Dotnet

Description:
This PR does the following -

  • changed the channel for the test from LTS to 8.0 to retrieve the latest sdk version 8.0.302

Changelog:

  • Updated install_dotnet_lts.sh
    • changed channel to 8.0

Checklist:

  • It should pass the test as the test install latest version 8.0.302

@@ -6,7 +6,7 @@
#
# Docs: https://github.com/devcontainers/features/tree/main/src/dotnet
# Maintainer: The Dev Container spec maintainers
DOTNET_VERSION="${VERSION:-"latest"}"
DOTNET_VERSION="${VERSION:-"lts"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this was just for testing but otherwise I wouldn't change the default version

@@ -105,7 +105,7 @@ done

# Install .NET versions and dependencies
# icu-devtools includes dependencies for .NET
check_packages wget ca-certificates icu-devtools
check_packages wget ca-certificates icu-devtools curl jq
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you could drop curl and only use wget for consistency with existing code

@@ -12,6 +12,9 @@ fetch_latest_version_in_channel() {
wget -qO- "https://dotnetcli.azureedge.net/dotnet/Runtime/$channel/latest.version"
elif [ "$runtime" = "aspnetcore" ]; then
wget -qO- "https://dotnetcli.azureedge.net/dotnet/aspnetcore/Runtime/$channel/latest.version"
elif [ "$channel" = "LTS" ]; then
echo $(curl -s https://dotnetcli.blob.core.windows.net/dotnet/release-metadata/releases-index.json |
Copy link
Contributor

Choose a reason for hiding this comment

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

I am thinking it would be good to replace all usages of latest.version with release-index.json for consistency, and because using the latest.version file in scripts is not endorsed by the .NET team

@@ -87,7 +87,7 @@
"remoteUser": "vscode",
"features": {
"dotnet": {
"version": "latest",
"version": "lts",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same remark, if not needed for testing, I would revert this.

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.

3 participants