Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin version of ltag for go <= v1.15 compatibility #445

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

Joffref
Copy link
Contributor

@Joffref Joffref commented Aug 30, 2022

Signed-off-by: Mathis Joffre [email protected]

Issue #, if available: #364

Sorry for multiple PR opening I had some troubles with my branches and squashing. This the final PR for this issue.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@Kern-- Kern-- left a comment

Choose a reason for hiding this comment

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

Thanks for this!

If v0.2.3 still works with newer versions of go, I would prefer we just pin it for all go versions. We're already looking at removing support for <=1.15 in #440 at which point we can update the pin.

@Joffref
Copy link
Contributor Author

Joffref commented Aug 30, 2022

I think it is the best option I will made changes, plus we should consider to pin a version for each dependency to avoid situation like this one.

Copy link
Contributor

@ginglis13 ginglis13 left a comment

Choose a reason for hiding this comment

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

small nit, otherwise LGTM

@@ -144,6 +144,6 @@ firecracker-clean:

lint: deps
gofmt -s -l .
$(FC_TEST_DATA_PATH)/ltag -check -v -t .headers
$(FC_TEST_DATA_PATH)/bin/ltag -check -v -t .headers
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(FC_TEST_DATA_PATH)/bin/ltag -check -v -t .headers
$(FC_TEST_BIN_PATH)/ltag -check -v -t .headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, it doesn't work like this in the linter section of buildkite.

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.

4 participants