docs: define go import convention #258
Conversation
|
I feel as this is unnecessary. This is automatically formatted with |
|
@cdrage it does not separate in the form of sections, so this is necessary and we have to do it manually. |
|
@surajssd Could we direct users to vim plugins / ide plugins instead? Using @kadel what do you think? Organizing something like imports seems unnecessary in terms of development, do we really have to have a doc to emphasize this? |
|
@cdrage yes golang plugins do sort the imports, but if you leave a line in between you end up forming sections in import section. And the tool does not remove those empty lines, it just forms section within that block, so we are trying to form a convention here. But if we agree on not putting any blank lines in imports then I am fine with it, but let's have something the tools are not good enough to do it on their own. |
|
Yeah, IMO this development.md is getting bigger and bigger and a lot of the stuff is either common sense (if you've gone through the Golang tutorials) or unnecessary such as these import functions. It has no effect on code and it still adheres to the |
surajssd
force-pushed
the
import-arrangement
branch
from
e28cd22
to
35e88d3
Compare
|
So there have been lot of instances #180 (comment) #179 (comment) #173 (comment) #242 (comment) where we had to point out in PRs saying that we are following the import conventions, because it was unnecessary diff in file even though that file had no other code change. And everytime I had to point to the old comment #149 (comment) where we decided this is how we are gonna settle on arranging imports. |
|
Ahhhhhhh that's why. Makes sense. You have a good point. @surajssd PR LGTM. |
docs/development.md
Outdated
|
|
||
| ### golang dependency import conventions | ||
|
|
||
| imports MUST be arranged in three sections, separated by empty line. |
There was a problem hiding this comment.
Imports
separated by an empty line
docs/development.md
Outdated
| for e.g. | ||
|
|
||
| ```go | ||
| "fmt" |
There was a problem hiding this comment.
may be better to unindent all of this
docs/development.md
Outdated
| thirdparty | ||
| ``` | ||
|
|
||
| for e.g. |
docs/development.md
Outdated
| "github.com/pkg/errors" | ||
| ``` | ||
|
|
||
| And once arranged in such sections let go tooling take care of sorting the |
There was a problem hiding this comment.
Once arranged, let gofmt sort the sequence of imports.
Would be a better sentence
| ```go | ||
| stdlib | ||
| kedge | ||
| thirdparty |
There was a problem hiding this comment.
We could also add a named import section here like done in https://github.com/kedgeproject/kedge/blob/master/pkg/spec/resources.go#L19-L38, this makes the imports look a lot cleaner.
There was a problem hiding this comment.
I think it is for knowing what imports are coming from where, also even if the imports are named or not the sorting happens the same way.
And same convention k8s also follows.
There was a problem hiding this comment.
You should use named imports only when there is a collision between multiple package names. Otherwise, it just adds confusion
Add a developer doc that explains how to arrange imports so that when new imports are added, unnecessary changes in import space are avoided.
surajssd
force-pushed
the
import-arrangement
branch
from
35e88d3
to
e854dbd
Compare
Add a developer doc that explains how to arrange imports so that
when new imports are added, unnecessary changes in import space
are avoided.
This was decided here #149 (comment) and we are following this since, wanted to make it official by putting it in docs.