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

feat(just): add brew-command-not-found setup #1874

Merged
merged 5 commits into from
Nov 26, 2024

Conversation

tulilirockz
Copy link
Collaborator

@tulilirockz tulilirockz commented Nov 3, 2024

This should setup command-not-found alongside brew-setup. The only thing missing is fish support that I couldnt set up yet (idk why it isnt working). Otherwise this seems good? Just needs a bit more testing. - Fixes #1327

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. brew Issues related to brew.sh integration size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Nov 3, 2024
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Nov 5, 2024
@m2Giles
Copy link
Member

m2Giles commented Nov 10, 2024

I think this might be a bit too much. Especially if this is going to install something from brew automatically. While brew is present, it is very much in an opt-in configuration.

@tulilirockz
Copy link
Collaborator Author

This probably should just be a justfile, right? Ill convert it over to that!

@tulilirockz tulilirockz force-pushed the brew-command-not-found branch 2 times, most recently from 7e2cf61 to 7b8c124 Compare November 10, 2024 17:24
@tulilirockz tulilirockz changed the title feat(brew): add command-not-found by default after setup feat(just): add brew-command-not-found setup Nov 10, 2024
@tulilirockz
Copy link
Collaborator Author

There you go! It seems to work fine on my machine

Copy link

@lem4s lem4s left a comment

Choose a reason for hiding this comment

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

As it is often quite easy to misunderstand the intention of the review comments let me make it clear here: There are absolutely no rhetorical questions in my comments and all questions are well intended and out of curiosity of understanding your intentions and learning a few things ;)

just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
just/bluefin-tools.just Outdated Show resolved Hide resolved
@tulilirockz
Copy link
Collaborator Author

As it is often quite easy to misunderstand the intention of the review comments let me make it clear here: There are absolutely no rhetorical questions in my comments and all questions are well intended and out of curiosity of understanding your intentions and learning a few things ;)

Most of the answers to your questions are: I just wrote bad code! Ill probably rewrite the just recipe with your review :p Thanks for reviewing!

@tulilirockz
Copy link
Collaborator Author

Im on my phone right now so I cant fix the recipe rn, but ill get on my computer, rewrite it and add some comments to clear up the logic

@lem4s
Copy link

lem4s commented Nov 13, 2024

Just added two comments on the review. Please check them. I can't seem to unresolve the issues or I am too blind to find the button... ;)

@m2Giles
Copy link
Member

m2Giles commented Nov 23, 2024

@tulilirockz resolve the merge conflict.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Nov 23, 2024
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Nov 23, 2024
@tulilirockz
Copy link
Collaborator Author

Should be good now - Fixed a few things that @lem4s mentioned, too

@tulilirockz

This comment was marked as outdated.

@tulilirockz
Copy link
Collaborator Author

image
Works without any changes now! - Hid the other comment to not cause confusion

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 26, 2024
@castrojo castrojo added this pull request to the merge queue Nov 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2024
@castrojo castrojo added this pull request to the merge queue Nov 26, 2024
Merged via the queue into ublue-os:main with commit 0444b09 Nov 26, 2024
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brew Issues related to brew.sh integration lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use brew's command-not-found
4 participants