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

Add package upgrade list #2093

Merged
merged 15 commits into from
Jan 21, 2024
Merged

Conversation

Doprez
Copy link
Contributor

@Doprez Doprez commented Jan 4, 2024

PR Details

There are some cases where nugets contain "Stride." but do not follow the same release version. This adds a list that can be used to check if the package should be upgraded.

This is a replacement for #1881

below is a test I did excluding Stride.Physics and Stride.Navigation showing that no other packages are effected but the ones not in the array.
image
image

Description

Adds a static array of valid projects and adds a couple checks for initial upgrades and sequential upgrades.

Related Issue

#2053

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@Doprez
Copy link
Contributor Author

Doprez commented Jan 4, 2024

Once this gets reviewed and hopefully merged if all looks well, I would like to do a demo video using the community toolkit with stride but this error is mildly annoying for me and Im sure if I advertise it many average users would deal with the same issue.

@VaclavElias
Copy link
Contributor

Maybe we should go with this solution "temporarily" (long term) till the Stride reservation is resolved and taken care of.

@Doprez
Copy link
Contributor Author

Doprez commented Jan 4, 2024

I think this list will still be useful for some edge cases but you are right that Stride reservation should be put in place. There were still some packages that had Stride but were not on the same version that I found in nuget.org.

https://www.nuget.org/packages/Stride.GraphX.WPF.Controls
https://www.nuget.org/packages/Stride.GNU.Gettext
https://www.nuget.org/packages/Stride.OpenTK
https://www.nuget.org/packages/Stride.Metrics

and a few more but these are some of the examples that may cause issues for adventerous nuget users.

@Doprez Doprez requested a review from Kryptos-FR January 6, 2024 22:01
@Eideren
Copy link
Collaborator

Eideren commented Jan 7, 2024

Xen2 sent a request for reserving the prefix over on nuget so I think it's safe to change from a whitelist to a blacklist instead; list only the specific repo under the prefix that should not be updated instead of listing those that should be. So that would be Stride.Community.*, Stride.Awesome.Shaders and the other you listed above

@Doprez
Copy link
Contributor Author

Doprez commented Jan 7, 2024

Xen2 sent a request for reserving the prefix over on nuget so I think it's safe to change from a whitelist to a blacklist instead; list only the specific repo under the prefix that should not be updated instead of listing those that should be. So that would be Stride.Community.*, Stride.Awesome.Shaders and the other you listed above

I changed over to an exclusion list like you said.

here is the quick test I did with the changes
exclusion list

image

result

image

@Doprez
Copy link
Contributor Author

Doprez commented Jan 7, 2024

@Eideren I addded a for loop to check the starts with validation as well. I couldn't quickly think of a better way than using ElementAt() since its a hashset and not a list I cant just index normally so I feel like this defeats the purpose of a hashset but I could be wrong.

@Eideren
Copy link
Collaborator

Eideren commented Jan 20, 2024

Sorry about the delay. Yep, just swap to an array, do an ordinal string equality and we're done :)

@Doprez
Copy link
Contributor Author

Doprez commented Jan 21, 2024

Not a problem at all, I think Im done with this PR now.

@Eideren Eideren merged commit b3f6b96 into stride3d:master Jan 21, 2024
2 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Jan 21, 2024

Thanks !

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

Successfully merging this pull request may close these issues.

4 participants