`resolvers.go` files are collapsed by default in Github review: problem and proposal

[frdzy]

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:

1. gqlgen's default behavior of adding the file notice string // Code generated to all generated files, unless omit_gqlgen_file_notice is configured in gqlgen.yml
2. linguist: this logic detects the string // Code generated as a marker for generated Golang code
3. 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).

[StevenACoffman]

Initially, the Resolvers may be completely generated by gqlgen. Subsequently, Resolvers may then transition to be only partially generated. At a still later point, Resolvers may then transition to be fully preserved by gqlgen and no longer maintained.

For Factors 2 and 3, I would appreciate it if you could file issues with both Linguist and GitHub that then link back here for reference. I have no real familiarity with or involvement with either project.

If you would like to add a PR here for a ResolverConfig option so that people could opt-in to customizing the generated notice for Resolvers this option would be both backwards compatible, and distinguish it from generated files that are not completely re-generated, and it could also allow people to adjust the comment to reflect the evolving status of the Resolvers during a project's lifecycle.

[HaraldNordgren]

This is very needed! Some very important changes (auth) will be in the resolvers and it's bad if the reviewer skips it.

For (all) intents and purposes resolvers.go is not a generated file in a useful sense.

[StevenACoffman]

For (all) intents and purposes, resolvers.go is not a generated file in any [sic] useful sense.

@HaraldNordgren You are entirely correct from your usage and perspective, and a PR as described above would be very welcome to you and those who like you have a similar usage pattern.

There are also some people whose usage pattern of gqlgen is to dynamically regenerate all resolvers every time, and they would like to preserve the viability of their current usage pattern, so it would be good to be sensitive to this as well.

[HaraldNordgren]

@StevenACoffman This would work (see https://github.com/github-linguist/linguist/blob/f101af52dce8d8a8da53556d197472327d61b753/lib/linguist/generated.rb#L338) although it's maybe a bit brittle:

https://github.com/99designs/gqlgen/pull/3877

Edit: Closing the PR since I found a solution.

[HaraldNordgren]

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:

@frdzy I have the same problem, and I'm starting to believe a problem could be the .gitattributes syntax, see here: https://github.com/github-linguist/linguist/blob/f101af52dce8d8a8da53556d197472327d61b753/test/test_repository.rb#L63

Maybe it should be:

/path_to_graphql/**/*.resolvers.go -linguist-generated

Edit: Confirmed that this works, however it's currently broken on the GitHub Preview mode so don't use that. (https://github.com/github-linguist/linguist/discussions/7661#discussioncomment-14881246)

[StevenACoffman]

@HaraldNordgren Can you clarify what your solution was? I had held off on merging your PR because you had mentioned it was brittle and it seemed like you were still doing further investigation.

[HaraldNordgren]

@StevenACoffman The solution is to turn off GitHub Preview.

[HaraldNordgren]

Both of these formats will work, as also confirmed with an engineer at GitHub:

*.resolvers.go -linguist-generated

*.resolvers.go linguist-generated=false

It could be that @frdzy also has an issue with the path /path_to_graphql/**/*.resolvers.go, not sure.

I use *.resolvers.go as a more general regex that works for me.

[StevenACoffman]

@HaraldNordgren Thank you! Where do you think this might be a good to place to mention this in our existing documentation?

---

To prevent generated resolver files from being collapsed by default during Github review, use a .gitattributes file to mark resolver files that match their filename and path pattern. A .gitattributes file uses the same rules for matching as .gitignore files. For more information, see PATTERN FORMAT in the Git documentation.

1. Unless the .gitattributes file already exists, create a .gitattributes file in the root of the repository.

2. Use the linguist-generated attribute to mark or unmark paths that you would like to be ignored for the repository's language statistics and hidden by default in diffs.

   For example, to unmark a path_to_graphql/my.resolvers.go file that would generally be considered generated, add this line to .gitattributes:

   *.resolvers.go linguist-generated=false
   

   An alternative format to unmark a file that would generally be considered generated, is to add this line to .gitattributes:

   *.resolvers.go -linguist-generated
   

[HaraldNordgren]

@StevenACoffman Maybe at the bottom before "Other resources"?

Looks good, but I managed to get GitHub to change their docs, so the suggested format is now only this:

*.resolvers.go -linguist-generated

See https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github