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

Made inits public for NoArguments and PageInfo #126

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

cshadek
Copy link
Contributor

@cshadek cshadek commented Oct 13, 2023

Addresses #124 to make inits public for both PageInfo and NoArguments. This should help with custom connections and enable people to call resolvers with no arguments directly.

@cshadek
Copy link
Contributor Author

cshadek commented Oct 13, 2023

@NeedleInAJayStack I don't know why macOS is failing, but I also fixed swiftformat warnings in ConnectionTests. The only real changes I made are making the inits public.

Copy link
Member

@NeedleInAJayStack NeedleInAJayStack left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @cshadek!

It appears that the MacOS failure is related to codeclimate: https://github.com/GraphQLSwift/Graphiti/actions/runs/6512886544/job/17691405715?pr=126#step:4:7956
@paulofaria, do you have any insight or would you prefer if we disable the codeclimate integration?

@cshadek
Copy link
Contributor Author

cshadek commented Oct 21, 2023

@NeedleInAJayStack, @paulofaria is it possible to approve this merge even with macOS failing?

@paulofaria
Copy link
Member

Is codeclimate still being used for test coverage? I don't remember if test coverage checks are required by the SSWG maturity level we're going for?

@cshadek
Copy link
Contributor Author

cshadek commented Oct 25, 2023

Is it possible to expedite this? We are trying to have custom connections in addition to the graffiti ones, and both need access to PageInfo. We use other libraries (Pioneer, PioneerRedisPubsub, etc), using the forked version seems to cause errors because both Pioneer and our other package are using the same identifier for different repos.

@NeedleInAJayStack
Copy link
Member

@cshadek Sure, we can merge this in since the CI shows that no tests are failing on MacOS. If you have any free cycles, we'd sure love some help either fixing the codeclimate CI or finding a different test coverage solution (I've been super slammed with other things recently). But yeah, don't want it to hold you up in the meantime.

@NeedleInAJayStack NeedleInAJayStack merged commit 09b3c37 into GraphQLSwift:main Oct 25, 2023
5 of 6 checks passed
@cshadek
Copy link
Contributor Author

cshadek commented Oct 26, 2023

@NeedleInAJayStack Thanks! I'll look into it. Not too familiar with codeclimate, but I'll see what I can do!

@cshadek cshadek deleted the public_no_arguments branch December 1, 2023 20:13
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.

None yet

3 participants