Prefer to explicitly return nil when there is no error for code readability #536

Closed
gmlewis opened this Issue Jan 31, 2017 · 6 comments

Projects

None yet

3 participants

@gmlewis
Member
gmlewis commented Jan 31, 2017

A common pattern in this repo is:

if err != nil {
  return ..., err
}
return ..., err

and we would like to change the last return to explicitly return ..., nil for Go readability.

@sahildua2305
Collaborator

Oh! It's going to be a lot of work. I think we should split it up.

Also, we should have a static check for this, no?

@shurcooL
Collaborator
shurcooL commented Feb 1, 2017 edited

Oh! It's going to be a lot of work. I think we should split it up.

There can be multiple smaller PRs that help resolve this issue, if that's easier. It doesn't have to be just 1 change. A lot of the recent PRs, including #535, have helped this issue already.

Also, we should have a static check for this, no?

I think @dominikh's staticcheck may be able to detect that err variable being returned has a constant nil value in the future.

@gmlewis
Member
gmlewis commented Feb 1, 2017

I believe that @willnorris wants to keep the dependencies of go-github extremely small, and most likely will not agree to pull in a new dependency for staticcheck. He can certainly correct me if I've misinterpreted our conversations, though.

@shurcooL
Collaborator
shurcooL commented Feb 2, 2017 edited

@gmlewis, there is no reason to pull in staticcheck as a dependency of go-github. It's simply a tool similar to go vet that might get a check in the future that would detect this type of issue, and make it easier for us (the developers/contributors to go-github) to find and resolve any occurrences.

@gmlewis
Member
gmlewis commented Feb 2, 2017

Ah, thank you, @shurcooL! This is excellent news. I need to check that out.

@sahildua2305 sahildua2305 added a commit to sahildua2305/go-github that referenced this issue Feb 2, 2017
@sahildua2305 sahildua2305 [improvement] Return nil when no error
Fixes #536 partitially
71f5cd3
@shurcooL shurcooL closed this in #546 Feb 13, 2017
@shurcooL
Collaborator

We're very close, but this is not fully resolved yet, see #546 (comment).

@shurcooL shurcooL reopened this Feb 13, 2017
@shurcooL shurcooL closed this in #550 Feb 15, 2017
@shurcooL shurcooL added a commit that referenced this issue Feb 15, 2017
@nspragg @shurcooL nspragg + shurcooL Return nil error explicitly when there is no error. (#550)
Follows #546.
Fully resolves #536.
dfd20fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment