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

Fix NuGet API calls in install script #287

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Conversation

davidxuang
Copy link
Contributor

@davidxuang davidxuang commented Oct 29, 2024

Changes per title.

@JoonghyunCho
Copy link
Member

Hello, I don't see the reason the package ID has to be the lower case. Any reason for this change?

@davidxuang
Copy link
Contributor Author

It's failing.

图片

@davidxuang
Copy link
Contributor Author

Is this being reviewed? This does not affect development much because it can be bypassed if you name a version explicitly when executing the script, but it's not practical to do this with CI every time .NET receive a new minor series.

@JoonghyunCho
Copy link
Member

sorry for the late review.
I still don't see the reason this should be changed. To me, it looks just an additional job in the script.
Additionally, as far as I know, the NOT ToLower() version is the exact url.
I don't understand the original url fails on your side.

@davidxuang
Copy link
Contributor Author

sorry for the late review. I still don't see the reason this should be changed. To me, it looks just an additional job in the script. Additionally, as far as I know, the NOT ToLower() version is the exact url. I don't understand the original url fails on your side.

For no way the current implementation is working.

图片

@JoonghyunCho
Copy link
Member

Ok, so you mean it could be failing in some CI or other system that probably handle it differently.
I will merge it and see if this could solve the issue. Thanks for the contribution.

Copy link
Member

@JoonghyunCho JoonghyunCho left a comment

Choose a reason for hiding this comment

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

thanks

@JoonghyunCho JoonghyunCho merged commit 74bd664 into Samsung:main Nov 21, 2024
2 checks passed
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.

2 participants