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

fix(bats): Add bats-file #19

Merged
merged 2 commits into from
Sep 20, 2023
Merged

Conversation

julienloizelet
Copy link
Collaborator

The Issue

"bats-file" helper was not installed.

And "bats-core" has been renamed in "bats-support" (see here ), so it is unnecessary to "brew install" it:

Important: bats-core has been renamed to bats-support. GitHub automatically redirects all references, e.g. submodules and clones will continue to work, but you are encouraged to update them. Version numbering continues where bats-core left off.

How This PR Solves The Issue

Install "bats-file" and remove "bats-core" install

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@julienloizelet julienloizelet requested a review from rfay September 19, 2023 06:26
action.yaml Outdated
@@ -64,7 +64,7 @@ runs:
run: |
# For bats-assert and friends
brew tap kaos/shell
brew install bats-core bats-assert bats-support jq mkcert yq
brew install bats-file bats-assert bats-support jq mkcert yq
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
brew install bats-file bats-assert bats-support jq mkcert yq
brew install bats-core bats-file bats-assert bats-support jq mkcert yq

I don't think you're correct. There are SO MANY THINGS in SO MANY PLACES here.

I find it mind boggling. And then the brew tap is kaos? ???

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is not clear at all.
And if you read here: https://github.com/ztombol/bats-docs#homebrew

Note: The required dependencies, bats-support as well as bats from the core tap will be installed automatically for you, with any of the two previous commands.

suggesting that we don't have to specify "brew install bats-support" ...

To keep it simple, I will do as you suggested in your review and run ALL install commands
brew install bats-core bats-file bats-assert bats-support jq mkcert yq

It shouldn't do any harm.

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

I'm baffled about all this, but have been for some time. But now the thing about bats-support makes you wonder if there's been some drama, etc.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks!

@rfay rfay changed the title fix(bats): Add bats-file and remove useless bats-core fix(bats): Add bats-file Sep 20, 2023
@julienloizelet julienloizelet merged commit 46e6ce0 into ddev:main Sep 20, 2023
@julienloizelet julienloizelet deleted the fix/bats-helper branch September 30, 2024 05:33
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.

2 participants