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 NewNullDecimal #234

Merged
merged 1 commit into from Oct 12, 2021
Merged

add NewNullDecimal #234

merged 1 commit into from Oct 12, 2021

Conversation

rubensayshi
Copy link
Contributor

Since NullDecimal{d, true} is discouraged (and static analysis tools like go vet will alert about it) adding NewNullDecimal seems like a nice way make code a bit more readable

nd := NewNullDecimal(d)
// vs
nd := NullDecimal{Decimal: d, Valid: true}

@mwoss
Copy link
Member

mwoss commented Oct 12, 2021

Sounds like a good idea. Thanks for your contribution @rubensayshi!

Copy link
Member

@mwoss mwoss left a comment

Choose a reason for hiding this comment

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

Overall looks good to me :3

decimal_test.go Outdated Show resolved Hide resolved
decimal_test.go Outdated Show resolved Hide resolved
@rubensayshi
Copy link
Contributor Author

awesome, I fixed the 2 FailNow -> Errorf

@mwoss
Copy link
Member

mwoss commented Oct 12, 2021

Looks great tho. Thanks for the contribution once again @rubensayshi

@mwoss mwoss merged commit 3259e0a into shopspring:master Oct 12, 2021
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.

None yet

2 participants