search

haproxytech / haproxy-consul-connect

HaProxy Connector for Consul Connect. Enables Service Mesh with Consul and HaProxy using TLS and Consul Discovery
Apache License 2.0
95 stars 20 forks source link

CLEANUP: environment-check: dependent executables error logic refactored #55

Closed amelhusic closed 4 years ago

amelhusic commented 4 years ago

Since only one error is captured, there is no need for make([]error, 2) and index logic. Instead, last error is captured and returned.

Additionally, sync.WaitGroup exists, but there was no goroutines spawned. So, added go keyword to spawn goroutines.

ShimmerGlass commented 4 years ago

I just left a small nitpick comment since the goal is cleaning up. LGTM

ShimmerGlass commented 4 years ago

I just noticed that the original code for this is wrong. The lambda hardcodes the version number instead of using the argument and strings.Compare won't work for versions like 0.1.10 > 0.1.2. This is not the scope of this PR tho, I can fix it after if you want. Overwise LGTM :)

amelhusic commented 4 years ago

I found there is a library for comparing versions: https://github.com/hashicorp/go-version. If you wish, I can open another PR to address this issue?

ShimmerGlass commented 4 years ago

Sure :)