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

remove: disgo.OS #366

Merged
merged 1 commit into from
Jun 24, 2024
Merged

remove: disgo.OS #366

merged 1 commit into from
Jun 24, 2024

Conversation

@topi314
Copy link
Member

topi314 commented Jun 24, 2024

What is the reason for this change?

@Malix-Labs
Copy link
Contributor Author

Malix-Labs commented Jun 24, 2024

runtime.GOOS already output the desired output from disgo.getOS() while making it even more portable

See updated https://github.com/disgoorg/disgo/pull/366#issue-2370847615#references

@Malix-Labs
Copy link
Contributor Author

Malix-Labs commented Jun 24, 2024

Also, disgo.OS will also become redundant with runtime.GOOS

@Malix-Labs Malix-Labs changed the title enhance: refactor disgo.OS deprecate: redundant disgo.getOS Jun 24, 2024
@topi314
Copy link
Member

topi314 commented Jun 24, 2024

while I can't exactly tell you why this was introduced in the first place since it's been over 2y lol, it was prob to make it easily overwritable, the structure has been quite different back then and this seems to has stuck with us since.

These days you can overwrite this differently so it's prob fine to get completely rid off the const & replace it with runtime.GOOS in https://github.com/disgoorg/disgo/blob/master/disgo.go#L108

@Malix-Labs Malix-Labs changed the title deprecate: redundant disgo.getOS deprecate: disgo.OS Jun 24, 2024
@Malix-Labs
Copy link
Contributor Author

Done

@topi314 topi314 changed the title deprecate: disgo.OS remove: disgo.OS Jun 24, 2024
@topi314 topi314 merged commit 2bb7320 into disgoorg:master Jun 24, 2024
3 checks passed
@Malix-Labs Malix-Labs deleted the patch-1 branch June 24, 2024 20:10
@topi314
Copy link
Member

topi314 commented Jun 24, 2024

thanks

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

Successfully merging this pull request may close these issues.

2 participants