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

Unpin CF cli #3702

Merged
merged 2 commits into from
Jan 13, 2025
Merged

Unpin CF cli #3702

merged 2 commits into from
Jan 13, 2025

Conversation

danail-branekov
Copy link
Member

Is there a related GitHub Issue?

#3649

What is this change about?

  • Revert "Temporarily pin cf cli version to 8.8.3"
  • Enable globstar option when running bash commands

Does this PR introduce a breaking change?

No

Tag your pair, your PM, and/or team

@georgethebeatle @uzabanov

Unpin cf cli, latest cli github release has been fixed to contain cli
binaries

fixes #3649

This reverts commit c3efa00.
`clean-bin` target does not clean the `./bin` directory otherwise
@georgethebeatle georgethebeatle merged commit 1e575be into main Jan 13, 2025
10 checks passed
@georgethebeatle georgethebeatle deleted the unpin-cf-cli branch January 13, 2025 14:12
@cniles
Copy link
Contributor

cniles commented Jan 13, 2025

@georgethebeatle @danail-branekov, a thought on the setting use of the globstar option. This breaks compatability with versions of bash older than 4.0. MacOS doesn't come bundled with bash 4.0 for licensing reasons. Its simple enough to alter/disable it or even install bash 4.0 through homebrew, but thats a minor irritance. Maybe its worth considering an alternative that doesn't use the globstar feature? It looks like the only line that uses it is to clean; an alternative maybe find ./ -d -name bin -exec rm -rf {} \; ?

@danail-branekov
Copy link
Member Author

Fair point @cniles

Do you know whether Korifi's Makefile is compatible with older bash versions? If not, we should maybe not bother about it. And I really would not want to downgrade my dev setup to test it.

We use recent bash versions (bash 4 has been released sixteen years ago!) and we do not consider supporting such old versions. But if that is a low-hanging fruit, we could totally do that.

@cniles
Copy link
Contributor

cniles commented Jan 14, 2025

Thanks for the response. All the make targets work. I feel slightly silly for bringing it up now and take your point of how old it is 😳, but I do think it could potentially make the local dev setup process less abrasive.

This command appears to have an equivalent effect of removing all the binaries while preserving bin directories:

find . -wholename '*/bin/*' -delete

@danail-branekov
Copy link
Member Author

It is a bit funny that today a colleague of mine complained that make does not work on his mac anymore :)

Would you be willing to PR a change with the command you pointed out - you would get some contributions credits! I would only request to put a comment on the command why globs are not used - we would probably "fix" it several months later after we have forgotten about this conversation :)

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