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 redundant ToQuantity methods #791

Conversation

AKTheKnight
Copy link
Contributor

This PR removes the ToQuantity methods that accept an int as a parameter and leaves the ones that accept a long.

As all the methods call the same method linked below, that only accepts a long, we already box everything down to a long to begin with:

private static string ToQuantity(this string input, long quantity, ShowQuantityAs showQuantityAs = ShowQuantityAs.Numeric, string format = null, IFormatProvider formatProvider = null)

Also remove unneeded AssertEquals that box ints to longs

We don't need int specific methods, when we box it all down to a long for the conversions anyway.
Also remove unneeded AssertEquals
@clairernovotny clairernovotny added this to the v3.0 milestone Mar 4, 2019
@clairernovotny
Copy link
Member

While this is code compatible, it is a binary breaking change. It can go in for the next major release.

@AKTheKnight AKTheKnight deleted the remove-redundundant-toquantity-methods branch March 16, 2019 11:13
@AKTheKnight AKTheKnight restored the remove-redundundant-toquantity-methods branch March 16, 2019 11:52
@AKTheKnight
Copy link
Contributor Author

Oops, didn't mean to close this PR.

@AKTheKnight AKTheKnight reopened this Mar 16, 2019
@SimonCropp SimonCropp removed this from the v3.0 milestone Feb 14, 2024
@SimonCropp
Copy link
Collaborator

@AKTheKnight i raised and merged a new PR. to save you the merge conflicts. And i credited you on the commit

will close this one

@SimonCropp SimonCropp closed this Feb 14, 2024
@AKTheKnight AKTheKnight deleted the remove-redundundant-toquantity-methods branch February 14, 2024 12:23
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.

3 participants