apigee / registry

The Registry API allows teams to track and manage machine-readable descriptions of APIs.
https://apigee.github.io/registry
Apache License 2.0
145 stars 33 forks source link

Line breaks in function declarations violate private style guidance #803

Open timburks opened 1 year ago

timburks commented 1 year ago

Discussions in code reviews (example) have pointed us to private guidance indicating that line breaks in function declarations are bad Go style.

The guidance can be summarized as "The signature of a function or method declaration should remain on a single line to avoid indentation confusion." Unfortunately I've been unable to find a public discussion of this.

An example violation is below.

func applyApiVersionPatch(
    ctx context.Context,
    client connection.RegistryClient,
    version *models.ApiVersion,
    parent string) error {

Guidance claims that the following is more readable and preferred:

func applyApiVersionPatch(ctx context.Context, client connection.RegistryClient, version *models.ApiVersion, parent string) error {

However, this (and similar declarations) suffers from excessive line length and obscures the number and sequence of arguments. Closer adherence to guidance to have short (one-character?) variable names might improve this, but possibly at the cost of overall readability.

Currently we have 46 instances of this style violation:

~/registry$ find . -name "*.go" -exec egrep "func .*\($" {} \; | wc -l
46

A similar count finds over 700 instances of this style violation in Kubernetes:

~/kubernetes-1.25.3$ find . -name "*.go" -exec egrep "func .*\($" {} \; | wc -l
702

There are 20 violations in Hugo:

~/hugo-0.105.0$ find . -name "*.go" -exec egrep "func .*\($" {} \; | wc -l
20

There are also a few violations in the Go source code, with three directly linked below:

theganyo commented 1 year ago

I think this is bikeshedding and we should close this.