Skip to content

Conversation

@amelhusic
Copy link
Collaborator

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.

}
func() {
ensureVersion(haproxyBin, "^HA-Proxy version ([0-9]\\.[0-9]\\.[0-9])", "2.0", 0)
go func() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: this can be just go ensureVersion(...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks :) Updated

@ShimmerGlass
Copy link
Collaborator

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

@ShimmerGlass
Copy link
Collaborator

ShimmerGlass commented May 20, 2020

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
Copy link
Collaborator Author

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
Copy link
Collaborator

Sure :)

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.
@aiharos aiharos merged commit eaa0576 into haproxytech:master May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants