Conversation
7f117e7 to
cfd9efc
Compare
63c79c4 to
847c89f
Compare
pkg/parsers/filters/parse.go
Outdated
pkg/parsers/parsers.go
Outdated
There was a problem hiding this comment.
The last part of this is a bit difficult to understand. I think I mistakenly gave you a bad recommendation here. Maybe "ParseUnixAddr parses and validates that the specified address is a valid UNIX socket address. It returns a formatted UNIX socket address, either using the address parsed from addr, or the contents of defaultAddr if addr is a blank string".
Totally unrelated to this PR, but ParseHost has a silly line:
addr = fmt.Sprintf("%s", defaultTCPAddr)
|
Thanks so much for taking the time to incorporate my feedback. The new diff looks really good. I pointed out a few minor things but it's almost ready to go. |
|
LGTM from a Windows perspective. FYI, #14838 will change the winconsole stuff, so I would hold on that for a separate PR. |
|
@jhowardmsft sounds good to me for I'm wondering something, which one is better between : type VersionInfo struct {
Kernel int // Version of the kernel (e.g. 4.1.2-generic -> 4)
Major int // Major part of the kernel version (e.g. 4.1.2-generic -> 1)
Minor int // Minor part of the kernel version (e.g. 4.1.2-generic -> 2)
Flavor string // Flavor of the kernel version (e.g. 4.1.2-generic -> generic)
}and: type VersionInfo struct {
// Version of the kernel (e.g. 4.1.2-generic -> 4)
Kernel int
// Major part of the kernel version (e.g. 4.1.2-generic -> 1)
Major int
// Minor part of the kernel version (e.g. 4.1.2-generic -> 2)
Minor int
// Flavor of the kernel version (e.g. 4.1.2-generic -> generic)
Flavor string
} |
pkg/proxy/tcp_proxy.go
Outdated
There was a problem hiding this comment.
What is the role of a TCPProxy? When should it be used? As a resource, does it require management?
There was a problem hiding this comment.
I'm tempted to say something like on TCPListener (http://golang.org/src/net/tcpsock_posix.go?s=7230:7268#L221) as it's mostly meant to be used as proxy.NewProxy and the function will figure out if it should use TCP or UDP.
As a resource, does it require management?
Not sure at all about that 😓.
cd89ac7 to
5087924
Compare
|
Rebased and added some simple package documentation. |
b7db20d to
29dfa38
Compare
|
needs rebase sorry |
|
feel free to ping me after for merge |
- pkg/useragent - pkg/units - pkg/ulimit - pkg/truncindex - pkg/timeoutconn - pkg/term - pkg/tarsum - pkg/tailfile - pkg/systemd - pkg/stringutils - pkg/stringid - pkg/streamformatter - pkg/sockets - pkg/signal - pkg/proxy - pkg/progressreader - pkg/pools - pkg/plugins - pkg/pidfile - pkg/parsers - pkg/parsers/filters - pkg/parsers/kernel - pkg/parsers/operatingsystem Signed-off-by: Vincent Demeester <vincent@sbr.pm>
|
@jfrazelle rebased, let's hope janky is a little happier this time 😝. |
|
LGTM effing dev/urandom errors on ci, unrelated |
golint on some pkg/* packages
|
🎉 |
I started to work on
golintthepkg/*package 🐸.Few TODOs :
see comments below.pkg/term/winconsoleThis PR is part of #14756 🐧. It also covers a bit of #11578.
Signed-off-by: Vincent Demeester vincent@sbr.pm