-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Remove some unnecessary import renames. #312
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: monopole The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
9c430e7
to
c5f5f12
Compare
c5f5f12
to
340cb2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC if there is a -
in the package name, goimports
linter will ask for an name alias.
@@ -17,7 +17,7 @@ limitations under the License. | |||
package patch | |||
|
|||
import ( | |||
yamlpatch "github.com/krishicks/yaml-patch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC if you remove this goimports
linter will complain about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely true. That's why i disabled the goimports check in pre-commit.sh
a while back (that's why this PR is passing).
goimports is broken for the case where the actual location of code has a hyphen, but the code itself has a package name that doesn't have a hyphen, e.g.
location: https://github.com/krishicks/yaml-patch
package name: https://github.com/krishicks/yaml-patch/blob/94f42521ff99ba62ddeca5ddae47264ade0b8df6/patch.go#L1
There's confusing discussion around this issue in golang/go#17546
I want to remove these aliases because the goland linter is (correctly) flagging them as unnecessary, and seeing these reports is adding noise to the report, making it more difficult to see real problems.
/lgtm |
and an unnecessary creation of empty struct