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

osversion: implement stringer interface, deprecate ToString() #1547

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

thaJeztah
Copy link
Contributor

This allows the type to be used with fm.Sprintf() and similar uses.

To preview the GoDoc (pkg.go.dev);

GOOS=windows pkgsite -http=:6060 .

Screenshot 2022-10-18 at 15 21 14

@thaJeztah thaJeztah requested a review from a team as a code owner October 18, 2022 13:22
@thaJeztah
Copy link
Contributor Author

/cc @tianon 😄

Copy link
Contributor

@jterry75 jterry75 left a comment

Choose a reason for hiding this comment

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

LGTM. Super funny. I had no idea this wasnt already the String interface. Likely @lowenna's fault haha

@jterry75
Copy link
Contributor

BTW, nothing but love for you JH!

@thaJeztah
Copy link
Contributor Author

Super funny. I had no idea this wasnt already the String interface. Likely @lowenna's fault haha

Progressive insight! This came up in a chat with @tianon and I thought "let's fix that!" 😂

@jterry75
Copy link
Contributor

So funny

@thaJeztah
Copy link
Contributor Author

@anmaxvl @kevpar this one good to go?


func TestOSVersionString(t *testing.T) {
v := OSVersion{
Version: 123,
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, this should be 809042555 (123 | (2 << 8) | (12345 << 16)), but I think you can just set it to zero for this test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good one; yes I can set it to 809042555 (I wanted to avoid setting it to 0, as that's the default value, and such values could e.g. be ignored / omit-empty-like things).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍 ptal

This allows the type to be used with fm.Sprintf() and similar uses.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@jterry75
Copy link
Contributor

jterry75 commented Jan 6, 2023

/merge

@helsaawy helsaawy merged commit 276c435 into microsoft:main Feb 24, 2023
@thaJeztah thaJeztah deleted the stringer_all_the_strings branch February 24, 2023 19:16
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.

4 participants