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

Flip importas configuration #1959

Closed
wants to merge 1 commit into from
Closed

Conversation

cbandy
Copy link
Contributor

@cbandy cbandy commented May 7, 2021

The standalone importas tool takes arguments in the form {package}:{alias} and stores them in a map in which package is the key. The intent is to describe the one alias that is appropriate for a unique package path.

The golangci-lint configuration does the opposite; the alias is the key. This effectively "reserves" an alias for one package. This difference in configuration limits the number of packages that can be configured and doesn't align with the goals and documentation of the standalone tool.

This PR aligns our configuration with the standalone tool. It is a breaking change, but another breaking change to this linter's configuration is waiting to be released in ffe8061.

The standalone importas takes arguments in the form "{package}:{alias}"
and stores them in a map in which package is the key. The intent is to
describe the one alias that is appropriate for a unique package path.

This aligns our configuration with the standalone tool.
@boring-cyborg
Copy link

boring-cyborg bot commented May 7, 2021

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented May 7, 2021

CLA assistant check
All committers have signed the CLA.

@ldez
Copy link
Member

ldez commented May 7, 2021

Hello,

It's not possible to flip the configuration due to the configuration parsing system.
There is a bug when the key contains dot.

It's related to viper spf13/viper#324

@ldez ldez added blocked Need's direct action from maintainer won't fix This will not be worked on and removed blocked Need's direct action from maintainer labels May 7, 2021
@ldez ldez closed this May 7, 2021
@cbandy
Copy link
Contributor Author

cbandy commented May 7, 2021

I see. Are you open to changing it to a list of strings? These match how the standalone tool works exactly.

  importas:
    # if set to `true`, force to use alias.
    no-unaliased: true
    # List of aliases
    aliases:
      # using `servingv1` alias for `knative.dev/serving/pkg/apis/serving/v1` package
      - knative.dev/serving/pkg/apis/serving/v1:servingv1
      # using `autoscalingv1alpha1` alias for `knative.dev/serving/pkg/apis/autoscaling/v1alpha1` package
      - knative.dev/serving/pkg/apis/autoscaling/v1alpha1:autoscalingv1alpha1
      # You can specify the package path by regular expression,
      # and alias by regular expression expansion syntax like below.
      # see https://github.com/julz/importas#use-regular-expression for details
      - knative.dev/serving/pkg/apis/(\w+)/(v[\w\d]+):$1$2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
won't fix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants