Skip to content
This repository has been archived by the owner on Apr 14, 2024. It is now read-only.

Zsh + Remove core count + Brew install function #10

Merged
merged 9 commits into from
Jan 27, 2024
Merged

Zsh + Remove core count + Brew install function #10

merged 9 commits into from
Jan 27, 2024

Conversation

shinra-electric
Copy link
Contributor

@shinra-electric shinra-electric commented Jan 27, 2024

Made a few small changes:

  • Uses Zsh instead of Bash
  • Removes core count and -j${CORES} arg from building
  • Adds a function for installing brew dependencies [Needs Testing]

Reasons for the changes:

  1. Zsh is the default shell on macOS. It's not 100% compatible with Bash, but I tested the script and it works fine.
  2. According to this article Ninja automatically uses the maximum number of cores available. There is no need to check.
  3. This will check for a brew installation before using the brew install command. Output is cleaner but it seems a bit slow to me. Maybe there is a better way to check, or maybe the existing command is faster. Needs testing. Edit: Updated to check for presence in $(brew --prefix)/opt/ , it is fast now.

Checking for the install locally should in theory be faster than making a net request. 

Should be tested, seems a little bit slow for me. Maybe a different check might be better.
Zsh is the default shell on macOS, so use that instead of Bash. 

While Zsh is very similar to Bash, there are some incompatibilities. However I tested the script and it works fine.
According to [this article](https://spectra.mathpix.com/article/2024.01.00364/a-complete-guide-to-the-ninja-build-system) Ninja automatically uses the maximum amount of cores available. There is no need to check and set with -j${CORES}
@shinra-electric shinra-electric marked this pull request as draft January 27, 2024 13:59
@shinra-electric
Copy link
Contributor Author

The brew list command is slow... I'll try to think of a better one

it's slightly faster than just `brew list`
@shinra-electric shinra-electric marked this pull request as ready for review January 27, 2024 14:12
Simplify `"${deps[@]}"` to just `$deps[@]`
Tell the user that we are checking the homebrew dependencies
@shinra-electric
Copy link
Contributor Author

Ok, I'll stop fiddling with this....

@mavethee mavethee merged commit a0f3ad6 into mavethee:main Jan 27, 2024
@mavethee
Copy link
Owner

Thanks for PR! ;)

@shinra-electric shinra-electric deleted the zsh-script branch January 27, 2024 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants