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

Rename or migrate away from custom HashCode class #2909

Closed
TheCakeIsNaOH opened this issue Nov 18, 2022 · 3 comments
Closed

Rename or migrate away from custom HashCode class #2909

TheCakeIsNaOH opened this issue Nov 18, 2022 · 3 comments

Comments

@TheCakeIsNaOH
Copy link
Member

Is Your Feature Request Related To A Problem? Please describe.

Chocolatey CLI is using a custom HashCode implementation:
https://github.com/chocolatey/choco/blob/924a785abe806c0a61541b86d273f7643d8ed20e/src/chocolatey/infrastructure.app/utility/HashCode.cs

However, in dotnet core (2.1 I think), a HashCode class was added to dotnet itself. https://learn.microsoft.com/en-us/dotnet/api/system.hashcode

Therefore, once Chocolatey moves to dotnet, these classes will conflict, as they are both named HashCode

Describe The Solution. Why is it needed?

One option would be to rename that class to chocolatey.infrastructure.app.utility.HashCode or similar, and keep using it.

Another option would be to remove that class, and add the Microsoft.Bcl.HashCode package. It is built for netstandard 2.0, and so could be switched to immediately for release with v2.0

Neither of these would need to be done now, they could be done when migrating to dotnet from .Net framework. However, v2.0 may be a good opportunity to make this change.

Additional Context.

N/A

Related Issues

TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Nov 18, 2022
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Nov 23, 2022
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Dec 13, 2022
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2023
@gep13
Copy link
Member

gep13 commented Jan 12, 2023

@TheCakeIsNaOH thanks for raising this.

As discussed, there would be potentially some related work in CCM, before we could make this change. As such, although we are updating to .NET 4.8 in the next release, we will hold off on this particular change, until we can guarantee compatibility.

@gep13 gep13 added 0 - Backlog Requires Upstream Change Requires changes to a different location once issue is fixed or implemented and removed 0 - _Triaging labels Jan 12, 2023
@gep13 gep13 added this to the 2.0.0 milestone Mar 16, 2023
@gep13 gep13 self-assigned this Mar 16, 2023
@gep13
Copy link
Member

gep13 commented Mar 17, 2023

After some further investigation, it was found that it was possible to move forward with this change, so the steps have been taken to do this, both here, and in the Commercial components.

@gep13 gep13 closed this as completed Mar 17, 2023
@gep13 gep13 removed Improvement Requires Upstream Change Requires changes to a different location once issue is fixed or implemented labels Mar 21, 2023
@choco-bot
Copy link

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

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

No branches or pull requests

3 participants