Add import alias linting and standardize API imports#2142
Add import alias linting and standardize API imports#2142IrvingMg wants to merge 1 commit intoshipwright-io:mainfrom
Conversation
openshift-ci
bot
added
release-note-none
pull-request-size
bot
added
the
size/XXL
Signed-off-by: Irving Mondragón <mirvingr@gmail.com>
IrvingMg
force-pushed
the
standardize-go-import-aliases
branch
from
e64b593 to
5f834be
Compare
|
@shipwright-io/contributors PTAL |
There was a problem hiding this comment.
/approve
Thanks! This will help us a lot with code consistency. I think we had tried something in the past with a different tool, however it wasn't integrated with golangci-lint and thus wasn't enforced.
My suggestions here are opinions and IMO shouldn't block merge/approval. I think we should have a time-scoped discussion (1 week, maximum) on the rules we want for import ordering and aliases. Otherwise this sort of exercise becomes a "paint the bikeshed" ordeal.
| importas: | ||
| alias: | ||
| - pkg: github.com/shipwright-io/build/pkg/apis/build/v1beta1 | ||
| alias: buildapi |
There was a problem hiding this comment.
| alias: buildapi | |
| alias: buildv1beta1 |
There was a problem hiding this comment.
-1
The idea of using buildapi / pipelineapi is that when you upgrade to a new version, you only change the import and potentially breaking changes. If the import alias also changes, then you have a lot other changes in code and cannot see the trees in the forest when you review this.
| - pkg: github.com/shipwright-io/build/pkg/apis/build/v1beta1 | ||
| alias: buildapi | ||
| - pkg: github.com/shipwright-io/build/pkg/apis/build/v1alpha1 | ||
| alias: buildapialpha |
There was a problem hiding this comment.
| alias: buildapialpha | |
| alias: buildv1alpha1 |
There was a problem hiding this comment.
I'm not sure about including the version number. As @SaschaSchwarze0 mentioned, the goal is to reduce noise when API upgrades happen. buildapi should refer to the main API version.But maybe we could rename buildapialpha to something like buildapiexperimental (probably something shorter). WDYT?
| - pkg: github.com/shipwright-io/build/pkg/apis/build/v1alpha1 | ||
| alias: buildapialpha | ||
| - pkg: github.com/tektoncd/pipeline/pkg/apis/pipeline/v1 | ||
| alias: pipelineapi |
There was a problem hiding this comment.
| alias: pipelineapi | |
| alias: pipelinev1 |
| - standard | ||
| - default | ||
| - prefix(github.com/shipwright-io/build) |
There was a problem hiding this comment.
This is a good start - I have my own preferences wrt import ordering, however I don't want to block this PR on a never-ending bikeshed discussion.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
4 participants
Changes
Standardize Go import aliases and enforce them using
golangci-lint:importaslinter to enforce alias naming (buildapi, buildapialpha, pipelineapi)gciformatter to enforce import grouping (stdlib, external, internal)Related Issue
Fixes #1485
Type of PR
/kind cleanup
Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes