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

remove unused root_.gotpl file #2621

Closed
wants to merge 1 commit into from
Closed

Conversation

fiatjaf
Copy link
Contributor

@fiatjaf fiatjaf commented Apr 21, 2023

I was confused about the role of this file, but after a long time I realized it wasn't being used anywhere, so removing for clarity.

I have:

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

@coveralls
Copy link

Coverage Status

Coverage: 75.297% (+0.5%) from 74.824% when pulling dccebeb on fiatjaf:delete into f1f63b5 on 99designs:master.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Apr 21, 2023

Pretty sure it contains top-level definitions that should not be duplicated across the generated files for each schema file. https://github.com/99designs/gqlgen/blob/master/codegen/generate.go#L139

But I appreciate you poking around and looking for ways to clarify and improve things!

@fiatjaf
Copy link
Contributor Author

fiatjaf commented Apr 21, 2023

Hmm, that's true. Can you explain to me how that works? Because the generated!.gotpl file is almost exactly the same as root_.gotpl and that is what is being actually used in practice when generating schema files, no root_.gotpl. I've asked about this on Discord.

In practice I can delete root_.gotpl and everything works fine. That function is never called by gqlgen generate.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Apr 23, 2023

Sure, that is called by generatePerSchema here, which is called by GenerateCode here

So root_.gotpl is needed when you have a follow-schema layout for resolver implementations specified in your gqlgen.yml as in:

resolver:
  layout: follow-schema

But root_.gotpl is not needed when you have a single-file layout for resolver implementations specified as in:

resolver:
  layout: single-file

@StevenACoffman
Copy link
Collaborator

If you don't mind, I'm going to close this issue for now.

@fiatjaf fiatjaf deleted the delete branch April 23, 2023 23:07
@fiatjaf
Copy link
Contributor Author

fiatjaf commented Apr 23, 2023

Yes, that makes sense, but I'm still confused. I did successfully run the generate flow using follow-schema and that codepath was never reached.

Also, this speaks to my stupidity, but nowhere in the code I could find a reference to the generated!.gotpl file, but clearly that is the file that is being used in practice during code generation (because if you modify it while deleting root_.gotpl you get the notifications).

Also root_.gotpl and generated!.gotpl are almost exactly the same.

I'm asking these questions because I feel there is something wrong that can be cleaned up even though everything seems to be working perfectly, but I can't figure it out for myself. Please ignore me if the questions are too annoying.

@fiatjaf fiatjaf mentioned this pull request May 19, 2023
2 tasks
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.

3 participants