-
Notifications
You must be signed in to change notification settings - Fork 22
Internalize [2 of 4] #18
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge when 🍏
Build failures look legit 😕 |
public let data: DataType? | ||
public let errors: [GraphQL.ResponseError]? | ||
internal let data: DataType? | ||
internal let errors: [GraphQL.ResponseError]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be left public, they are the purpose of this struct (deserializing response into errors/data)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They aren't needed on any of the generated models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are/can be used by external code though. From the README
guard let response = try? GraphQLResponse<ExampleSchema.QueryRoot>(jsonData: response) else {
print("Invalid GraphQL response")
return
}
if let errors = response.errors {
for error in errors {
print("GraphQL error: \(error.message)")
}
}
if let data = response.data {
let user = data.user
print("\(user.firstName) \(user.lastName)")
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is GraphQLResponse
used for? I can't find any usage of it. Can we not just remove it entirely?
What this does
Coverts unnecessarily
public
methods tointernal
.Relies on #13