Home for "Go Code Review Comments"

[joe...@google.com]

Hello gophers,

I was addressing some golang/lint issues and I was thinking that the warnings that lint gives needs to have more hyperlinks to an explanation as to why. However, I find it strange that lint references a GitHub Wiki page, which has no protection against drive-by edits. I fear that as more hyperlinks are added, they will increasingly become dead over time when the lint tool does not match up with the "Go Code Review Comments" document.

Since lint is the canonical tool for checking for style issues, what are people's thoughts on committing the CodeReviewComments document into the golang/lint repo? 

Some advantages:

* This will protect against drive-by edits since commits need review.

* It is easier to keep the lint tool in-sync with the document, since they are in the same repo.

Overall, the lint tool needs a little bit more <3.

JT

[joe...@google.com]

The "Go Code Review Comments" article is https://github.com/golang/go/wiki/CodeReviewComments

[Jaana Burcu Dogan]

I would rather be in favor of moving it to golang.org. You may not depend on lint, but can still apply the guideline. The other advantage is that golang.org articles are more visible.

There are varying opinions on lint. I remember rsc@ telling that we shouldn't enforce it a while ago. We need a consensus on that.

[Russ Cox]

On Fri, Feb 3, 2017 at 6:07 PM, joetsai via golang-dev <golan...@googlegroups.com> wrote:

Hello gophers,

I was addressing some golang/lint issues and I was thinking that the warnings that lint gives needs to have more hyperlinks to an explanation as to why. However, I find it strange that lint references a GitHub Wiki page, which has no protection against drive-by edits. I fear that as more hyperlinks are added, they will increasingly become dead over time when the lint tool does not match up with the "Go Code Review Comments" document.

Since lint is the canonical tool for checking for style issues, what are people's thoughts on committing the CodeReviewComments document into the golang/lint repo? 

Please don't. Obviously the two are related, but lint is secondary, not primary. As the README says, "Golint makes suggestions for many of the mechanically checkable items listed in Effective Go and the CodeReviewComments wiki page." The wiki page does not exist to serve golint, nor is it unusable without golint. It doesn't belong in the lint repo.

Some advantages:

* This will protect against drive-by edits since commits need review.

Is this a real problem or a hypothetical one? Formal review of the CodeReviewComments doc is more process for something that is explicitly trying to avoid process (they're guidelines, not rules).

 

* It is easier to keep the lint tool in-sync with the document, since they are in the same repo.

In what ways do they drift out of sync today? You said something about dead hyperlinks, but I don't understand exactly what the links look like or how they might become dead. A note at the top asking people not to reword headings seems like it would be enough to avoid #foo changing.

 

Overall, the lint tool needs a little bit more <3.

Be careful. People get too attached to golint and they start treating it as some kind of infallible oracle. It's not, and it's not intended to be.

Russ

[Joe Tsai]

The wiki page does not exist to serve golint, nor is it unusable without golint. It doesn't belong in the lint repo.

Sound reasonable. 

Is this a real problem or a hypothetical one? Formal review of the CodeReviewComments doc is more process for something that is explicitly trying to avoid process (they're guidelines, not rules).

 

Seems to have happened recently. The recent discussion in "Code Review Comments: Declaring Empty Slices" was spurred by the fact that some guidelines were added without prior discussion. The fact that changes to the guidelines resulted in a discussion on go-dev is a form of process regardless of whether we're trying to avoid it or not.

In what ways do they drift out of sync today?

This part was more hypothetical out of fear of how easy the page was to edit. A simple warning to avoid changing #foo sounds fine.

 Be careful. People get too attached to golint and they start treating it as some kind of infallible oracle. It's not, and it's not intended to be.

My original statement was more for Go users than myself. The lint repo gets a fair amount of traffic (about the same as protobuf repo) with users requesting either new rules be added or existing rules be changed. It seems to me that users do care about the lint tool. If the tool is not something we want to continue supporting, then we should make that message clear.

JT