Skip to content

Conversation

@karol-kokoszka
Copy link
Collaborator

This PR updates Makefile so that docker env can be setup on macbook.


Please make sure that:

  • Code is split to commits that address a single change
  • Commit messages are informative
  • Commit titles have module prefix
  • Commit titles have issue nr. suffix

@karol-kokoszka karol-kokoszka marked this pull request as ready for review December 19, 2025 15:04
Copy link
Collaborator

@Michal-Leszczynski Michal-Leszczynski left a comment

Choose a reason for hiding this comment

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

I guess that fix(chore) commit prefix is a typo?

go install golang.org/x/tools/cmd/stress@v0.2.0 || echo "Warning: Failed to install stress"
go install github.com/pressly/sup/cmd/sup@v0.5.3 || echo "Warning: Failed to install sup"
go install github.com/go-swagger/go-swagger/cmd/swagger@v0.25.0 || echo "Warning: Failed to install swagger"
go install github.com/mikefarah/yq/v4@latest || echo "Warning: Failed to install yq"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that you bumped yq from v3.0.0-20201202084205-8846255d1c37 to latest.
Using latest is problematic when we want to backport something to older branch and execute CI for it. In some cases, go version used on this branch might be too old for the latest version of installed package. This results in failing CI. If this pkg needs to be bumped for the purposes of this PR, I would say it's better to bump it to the newest version available today, but not to the latest tag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But also, why is this bump needed? I thought that the problems with macos setup revolved around host network used in docker env, but this PR does not touch that at all. Is it not a problem anymore?

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