99designs / 99designs/gqlgen

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

Open
#3,763 10 comments 1 reaction 0 assignees View on GitHub
Dominant language
Go
Stars
10.8k
Forks
1.3k
Avg merge
2d 36m
Merged PRs (30d)
26

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:
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](https://github.com/github-linguist/linguist/blob/5fe9e5602ec8ac3d6ab1cdeb8762114a7b5544d2/lib/linguist/generated.rb#L334-L339) 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).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.