Skip to content

Updated NumberFormatter to match expected behavior #894

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

Merged
merged 8 commits into from
Mar 5, 2017
Merged

Updated NumberFormatter to match expected behavior #894

merged 8 commits into from
Mar 5, 2017

Conversation

DunnCoding
Copy link
Contributor

@DunnCoding DunnCoding commented Feb 24, 2017

Made some updates to the NumberFormatter class, now the usesSignificantFigures property will only be set if you want to set either minimum or maximum significant figures and will prevent setting numberStyle to .decimal overwriting the values you set for minimum or maximum significant figures.

I ran the tests and they all passed, I also added a few new tests and made the necessary changes to the old tests to account for this change.

@parkera
Copy link
Contributor

parkera commented Mar 3, 2017

@swift-ci please test and merge

@ianpartridge
Copy link
Contributor

TestFoundation/TestNSByteCountFormatter.swift:368: error: TestNSByteCountFormatter.test_isAdaptiveFalseZeroPadsFractionDigitsTrue : XCTAssertEqual failed: ("123 MB") is not equal to ("123.5 MB") -

@DunnCoding
Copy link
Contributor Author

Thanks Ian, I imagine it's due to the ByteCountFormatter is currently configured to work with how the NumberFormatter use to be prior to this update, and just needs updating to work this updated version of the NumberFormatter but I'll look into it to confirm and update as needed.

@DunnCoding
Copy link
Contributor Author

I've confirmed it's due to how the new NumberFormatter update works, I will update the ByteCountFormatter and add changes to a new commit. I'll also tidy up the ByteCountFormatter as a few bits can be removed with this update to NumberFormatter as well.

@DunnCoding
Copy link
Contributor Author

@parkera @swift-ci Changes have been added, I ran tests on mac and linux and both passed.

@parkera
Copy link
Contributor

parkera commented Mar 5, 2017

Cool, let me kick this off again.

@parkera
Copy link
Contributor

parkera commented Mar 5, 2017

@swift-ci please test and merge

@swift-ci swift-ci merged commit 36743bc into swiftlang:master Mar 5, 2017
@DunnCoding DunnCoding deleted the numFormatter branch March 6, 2017 08:58
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.

5 participants