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: improve ujust bluefin-cli #857

Merged
merged 13 commits into from
Jan 30, 2024
Merged

Conversation

m2Giles
Copy link
Member

@m2Giles m2Giles commented Jan 29, 2024

Enables the use of all of toolboxes in ublue-os/toolboxes
Dynamically fetches the Quadlets and makes systemd targets
Moves the inline ujust command to it's own script

Enables the use of all of toolboxes in ublue-os/toolboxes
Dynamically fetches the Quadlets and makes systemd targets
Moves the inline ujust command to it's own script
@m2Giles m2Giles requested a review from castrojo as a code owner January 29, 2024 21:20
@m2Giles m2Giles requested a review from HikariKnight January 29, 2024 21:27
@HikariKnight
Copy link
Member

HikariKnight commented Jan 29, 2024

I will have to head to bed (it is midnight here), but i will continue looking at this tomorrow unless someone else picks up where i left off.
This is all i found so far.

@HikariKnight
Copy link
Member

HikariKnight commented Jan 30, 2024

Would also recommend adding an exit in case the user cancels a selection (for example when selecting a container they can press ESC to cancel, this will return an empty string, this currently seems to lock the script bluefin-cli.sh up)

@m2Giles
Copy link
Member Author

m2Giles commented Jan 30, 2024

That seems smart. Each question should resolve to a variable. If a variable is empty bail out. Right now a SIGINT or empty string will put the script in a bad state.

I also should move all questions to occur before any execution. For the loops that might be an issue but trap would probably be sufficient to have a clean exit for those.

Copy link
Member

@HikariKnight HikariKnight left a comment

Choose a reason for hiding this comment

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

Looks good now

usr/libexec/bluefin-cli.sh Outdated Show resolved Hide resolved
usr/libexec/bluefin-cli.sh Outdated Show resolved Hide resolved
usr/libexec/bluefin-cli.sh Outdated Show resolved Hide resolved
usr/libexec/bluefin-cli.sh Outdated Show resolved Hide resolved
usr/libexec/bluefin-cli.sh Outdated Show resolved Hide resolved
@HikariKnight HikariKnight added this pull request to the merge queue Jan 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2024
@m2Giles m2Giles added this pull request to the merge queue Jan 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2024
@HikariKnight HikariKnight added this pull request to the merge queue Jan 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 30, 2024
@castrojo castrojo added this pull request to the merge queue Jan 30, 2024
@castrojo castrojo removed this pull request from the merge queue due to a manual request Jan 30, 2024
@castrojo castrojo merged commit c64c8fb into ublue-os:main Jan 30, 2024
30 checks passed
@m2Giles m2Giles deleted the m2giles-patch1 branch January 31, 2024 16:14
awesomekyle pushed a commit to awesomekyle/bluefin that referenced this pull request Apr 24, 2024
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