go-ping / ping

ICMP Ping library for Go

MIT License

1.31k stars 344 forks source link

Add ability to set TTL #186

Closed cemremengu closed 2 years ago

cemremengu commented 2 years ago

175 rebased (h/t @hamptonmoore)

CHTJonas commented 2 years ago

We're blocked on CircleCI because it's not picking this up for some reason and "Required statuses must pass before merging"...

haylinmoore commented 2 years ago

Sorry, I didn’t see my notifications. Cheers to see somebody rebased and hat this might get implemented 🎉

SuperQ commented 2 years ago

Yea, I don't know why CircleCI isn't seeing this PR.

SuperQ commented 2 years ago

Looks like the linter failed:

packetconn.go:42:35: Error return value of `(*golang.org/x/net/ipv6.genericOpt).SetHopLimit` is not checked (errcheck)
        c.c.IPv6PacketConn().SetHopLimit(c.ttl)
                                        ^
packetconn.go:45:30: Error return value of `(*golang.org/x/net/ipv4.genericOpt).SetTTL` is not checked (errcheck)
        c.c.IPv4PacketConn().SetTTL(c.ttl)
                                   ^

cemremengu commented 2 years ago

@SuperQ should I just ignore the errors, log them as warning or fail fast by checking and returning the error?

what would be the most suitable approach?

SuperQ commented 2 years ago

If they return an error, the WriteTo should return 0, err

cemremengu commented 2 years ago

@SuperQ could you please take look ? I think it got stuck again

CHTJonas commented 2 years ago

Well CircleCI is just wild... Anyway I'm merging, thanks!