Skip to content

Code safety improvements #105

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 3 commits into from
Aug 17, 2022

Conversation

NeedleInAJayStack
Copy link
Member

This MR removes a significant number of fatalErrors, force-unwraps, force-trys, and force-casts. This is to improve code safety and be more in compliance with the SSWG Technical Best Practices.

Some force operations remain, but they are mainly where throwing is not possible (top level literals), or in code mostly copied from a specific source. Examples of this include Encoder/Decoders that are based on Foundation's JSON coders, and Visitor based on the graphql-js reference implementation.

paulofaria
paulofaria previously approved these changes Aug 17, 2022
Copy link
Member

@paulofaria paulofaria left a comment

Choose a reason for hiding this comment

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

This looks great! We can merge this right away, but would also be nice to integrate swift-lint to enforce some of these rules. What do you think? I believe swift-lint also does formatting, but we can also integrate swift-format, if it works better. What is your experience?

@NeedleInAJayStack
Copy link
Member Author

Thanks Paulo!

I think integrating swift-format is a great idea. I've only used Nick Lockwood's SwiftFormat, not Apple's swift-format, but from initial runs the Apple one looks more than adequate. I'll make a follow-up PR that integrates it.

@NeedleInAJayStack NeedleInAJayStack merged commit 17b96ed into GraphQLSwift:main Aug 17, 2022
@NeedleInAJayStack NeedleInAJayStack deleted the fix/codeSafety branch August 17, 2022 17:03
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.

2 participants