Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add goimports linter for consistent imports ordering #816

Merged
merged 3 commits into from
Sep 18, 2022

Conversation

orpheuslummis
Copy link
Contributor

@orpheuslummis orpheuslummis commented Sep 16, 2022

Relevant issue(s)

Resolves #815

Description

Enablers goimports linter.

Sorts imports using goimports -local "github.com/sourcenetwork/defradb" -w .

https://pkg.go.dev/golang.org/x/tools/cmd/goimports is part of official go/x

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

automatic lint, make test, eye balling

Specify the platform(s) on which this was tested:

  • MacOS

@orpheuslummis orpheuslummis added ci/build This is issue is about the build or CI system, and the administration of it. action/no-benchmark Skips the action that runs the benchmark. labels Sep 16, 2022
@orpheuslummis orpheuslummis self-assigned this Sep 16, 2022
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YASSSS!

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @orpheuslummis. I made one quick request btw. Also I know this linter rule thankfully has autofix ability, so basically just doing make lint:fix will apply these changes right?

goimports:
# Put imports beginning with prefix after 3rd-party packages.
# It's a comma-separated list of prefixes.
# Default: ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment line can go, as there is one argument you can provide goimports anyway

Suggested change
# Default: ""
# Default: ""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the comment thanks

make lint:fix works, but goimports is not perfect - when the imports list is sorted strangely or in a bad shape it seems to fail at reaching the desired state.

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@orpheuslummis orpheuslummis force-pushed the orpheus/chore/import-ordering-linting branch from ae6a3e9 to 9b4e236 Compare September 18, 2022 13:06
@orpheuslummis orpheuslummis merged commit 0729e9c into develop Sep 18, 2022
@orpheuslummis orpheuslummis deleted the orpheus/chore/import-ordering-linting branch September 18, 2022 13:18
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. ci/build This is issue is about the build or CI system, and the administration of it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linter for imports ordering
3 participants