Skip to content

Replace precondition if with guard in NSNumber.encode. #1143

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 1 commit into from
Jul 31, 2017

Conversation

oc243
Copy link

@oc243 oc243 commented Jul 31, 2017

Previously NSNumber.encode would call NSUnimplemented when you try to
do a keyed coding. Whilst it is true that this wasn't implemented it was
due to not being possible rather than not yet implemented.

With this patch we call preconditionFailure which is more consistent
with the rest of the project. We also use a guard so can remove a level of
indentation.

Previously NSNumber.encode would call NSUnimplemented when you try to
do a keyed coding. Whilst it is true that this wasn't implemented it was
due to not being possible rather than not yet implemented.

With this patch we call preconditionFailure which is more consistent
with the rest of the project. We also use a guard so can remove a level of
indentation.
@alblue
Copy link
Contributor

alblue commented Jul 31, 2017

@swift-ci please test

@alblue
Copy link
Contributor

alblue commented Jul 31, 2017

LGTM. What do you think @ianpartridge @phausler ? NB when viewing the diffs, put ?w=1 at the end so it's less noisy ...

@ianpartridge
Copy link
Contributor

Yes I think this is fine. Thanks @oc243

@ianpartridge
Copy link
Contributor

@swift-ci please test and merge

@alblue
Copy link
Contributor

alblue commented Jul 31, 2017

Looks like an unrelated segfault:

/home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux/swift/utils/build-script-impl: line 261: 9831 Segmentation fault (core dumped) "$@"

@swift-ci please test and merge

@swift-ci swift-ci merged commit 8bb98e5 into swiftlang:master Jul 31, 2017
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