-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Problem
Currently, generated resolver files are collapsed by default during Github review.
Even after I added a pattern for those files into a .gitattributes
file with linguist-generated=false
, they still continue to be collapsed by default:
// .gitattributes
/path_to_graphql/**/*.resolvers.go linguist-generated=false
Analysis
This seems to be due to a combination of 3 factors:
- gqlgen's default behavior of adding the file notice string
// Code generated
to all generated files, unlessomit_gqlgen_file_notice
is configured ingqlgen.yml
- linguist: this logic detects the string
// Code generated
as a marker for generated Golang code - Github's implementation: this is the part I understand the least. Just from trial and error, it seems like Github is ignoring the
.gitattributes
file for the purpose of considering a file to NOT be generated
I think 3. is worth an issue filed directly to Github, and I also verified that the .gitattributes
file itself is written properly by locally running linguist:
$ gem install linguist
$ cd $REPO_ROOT && linguist path_to_graphql/FOO/BAR.resolvers.go
type: Text
mime type: text/plain
language: Go
# (does not display "generated: true")
The rest of this issue focuses on gqlgen's behavior.
Workaround
I've worked around this at the moment by setting omit_gqlgen_file_notice=true
, then using .gitattributes
to mark all the other gqlgen-generated files as linguist-generated=true
.
While this is not quite ideal for discovery when a repo member opens the file directly, it does help resolve the problem that the file is collapsed-by-default in Github's review process.
Proposal
Is it possible to tackle factor 1. of the analysis here, by changing the file notice behavior?
Considering that the generated resolvers.go
files contain stubs that are meant to be customized further, the default // Generated by
file notice feels not entirely semantically correct.
- a) Boolean param within
ResolverConfig
to change the notice to// Partially generated by
instead- This would be the most backwards compatible behavior
- b) Change the default behavior altogether, so that generated
resolvers.go
files display a file notice that starts with something different like// Partially generated by
- c) String param within
ResolverConfig
to allow optional customization of the generated notice even further
Overall, (b) would be the most risk as a breaking change, but could be worth aiming for in the longer term by being more semantically correct. One feasible approach is to start with (a) first to give users a period to explicitly opt-in or opt-out, and in a later version switch to (b).