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 choice of whether to use slice-of-struct-pointers vs slice-of-st… #719

Closed
wants to merge 1 commit into from
Closed

Made choice of whether to use slice-of-struct-pointers vs slice-of-st… #719

wants to merge 1 commit into from

Conversation

gissleh
Copy link
Contributor

@gissleh gissleh commented May 19, 2019

Related to: #715

Version 0.9 broke backwards compatibility with 0.8 in a way that required major refactoring by changing resolver arguments, generated struct fields and resolver return values that are slices of structs to slices of struct pointers. This PR adds a configuration option to use the old 0.8 behavior.

I have tested whether the configuration has its intended effects. If there is a more elegant way to do this or a more proper way to test it, please let me know.

I understand that this adds considerable complexity in that a bug might occur for one choice of this configuration option but not the other, so I completely understand if you reject this because you do not want to support it.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@gissleh
Copy link
Contributor Author

gissleh commented May 19, 2019

I took the time to update my resolvers to use the new format by copying into pointer arrays. I don't like the added bloat, but it wasn't as great a refactor workload as I expected.

I won't really need this anymore then, but I'm willing to put in the work if you're up for merging this after a few changes.

@mathewbyrne
Copy link
Contributor

mathewbyrne commented May 20, 2019

Thanks for your work, but we don't currently believe that making this configurable is something we want to maintain. This change has been to bring slice returns from resolvers in line with regular structs, which were changed to always be pointers in 0.8.

Good to hear that the refactor wasn't too troublesome. We hear you that copying into pointer arrays isn't ideal, but also remember that users have been doing the opposite for a long time now, as many services/storage layers return slices of pointers.

@gissleh
Copy link
Contributor Author

gissleh commented May 20, 2019

Fair enough. I'll close this PR then.

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