Skip to content

Suggest using goimports with -local flag

Jaime Martinez requested to merge add-goimports-local-guide into master

What does this MR do?

The secure team code style and format already suggests using goimports with the -local flag. We should try to use the same formatting on different Go projects.

@hphilipps stated this best in labkit!57 (comment 361990315)

I think we should decide on a common way how to format imports. While our official GO style guide doesn’t say anything about using -local or not, there is a special section by the Secure team for using this standard. Do we really want to have per-team go style guides? I’m not really opinionated about -local or not, but we should find a common way at least.

Some reasons why -local would be useful:


The secure team already enforces this in their projects and it's in their code style and format guidelines. I don't see why not just follow one guide instead of having one per team.


Per the Go CodeReviewComments#imports.

Imports are organized in groups, with blank lines between them. The standard library packages are always in the first group.

In my previous experience, there's usually 3 groups:

  1. standard library
  2. third-party packages
  3. local packages.

This helps visually to identify dependencies more easily, especially when there's tons of packages being imported.


This is not what vim-go uses by default, or VSCode, so why add this burden if it's not better and adds complexity to get started?

I agree with the fact that there's a burden however this contributes to the coding style we have in place. It's also super easy to setup with vim-go and VSCode

VSCode settings: (could be added to Workspace settings located in ".vscode/settings.json")

"go.formatOnSave": true, 
"go.formatTool": "goimports",
"go.formatFlags": [
    "-local=gitlab.com/gitlab-org/labkit"
],

vim-go:

let g:go_fmt_command = "goimports"
let g:go_fmt_options = "-local=gitlab.com/gitlab-org/labkit"

Related issues

labkit#13 (closed)

Author's checklist (required)

Do not add the feature, frontend, backend, ~"bug", or database labels if you are only updating documentation. These labels will cause the MR to be added to code verification QA issues.

When applicable:

Review checklist

All reviewers can help ensure accuracy, clarity, completeness, and adherence to the Documentation Guidelines and Style Guide.

1. Primary Reviewer

  • Review by a code reviewer or other selected colleague to confirm accuracy, clarity, and completeness. This can be skipped for minor fixes without substantive content changes.

2. Technical Writer

  • Optional: Technical writer review. If not requested for this MR, must be scheduled post-merge. To request for this MR, assign the writer listed for the applicable DevOps stage.

3. Maintainer

  1. Review by assigned maintainer, who can always request/require the above reviews. Maintainer's review can occur before or after a technical writer review.
  2. Ensure a release milestone is set.
  3. If there has not been a technical writer review, create an issue for one using the Doc Review template.
Edited by Jaime Martinez

Merge request reports

Loading