Conversation
bde5109 to
6df55fe
Compare
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
6df55fe to
3409de9
Compare
There was a problem hiding this comment.
I'm not sure why this has been aliased?
There was a problem hiding this comment.
By convention, packages are given lower case, single-word names; there should be no need for underscores or mixedCaps.
Emacs wasn't happy with the - and I assumed golint would not like it.
There was a problem hiding this comment.
Yeah, I don't think the go lexer can handle - in identifiers. Probably we shouldn't have called it go-ansiterm, but it's too late to fix that. This alias is perfectly reasonable.
There was a problem hiding this comment.
@vdemeester if golint is not complaining i would keep it not aliased import "github.com/Azure/go-ansiterm"
@jstarks what matters is the package name (in this case: package ansiterm) not the import path.
|
LGTM, but not tested (the docker integration-cli won't exercise this). Just the query about the odd-looking alias. |
|
LGTM. I suspect that the . import was added when go-ansiterm was split from winterm, but I'm not sure. It also seems strange that golint thinks that a legitimate go feature (import with .) is unacceptable. Aren't the go developers and gofmt opinioned enough already? :) But again, I don't really have a strong opinion here. |
|
@vdemeester nvm I can't wait :P LGTM |
Lint pkg/term/windows package
Linting
pkg/term/windowsas part of #14756 🐹. It was almost already completely linted except for the.import thatgolintdoes not like.I was going to do it with the rest but I don't have a ready-to-use windows at hand to try out wether I broke it or not, so I'm shamelessly using the hard worker Janky for that 😉.
🐸
/cc @jhowardmsft @icecrime @tiborvass @LK4D4
Signed-off-by: Vincent Demeester vincent@sbr.pm