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): cleanup and standardization of just recipes #1003

Merged
merged 20 commits into from
Mar 14, 2024
Merged

Conversation

HikariKnight
Copy link
Member

Drafting this so we can start a discussion.
Unlike the just reorg done in ublue-os/bazzite#765
Bluefin has way less just recipes so I would like to prod and hear what the reorg needs will be and if the just files should be split up in groups like we did in bazzite.

In bazzite we focused on merging together recipes that involved the same thing under the configure- and setup- verbs.

  • configure for things that already exist on the image
  • setup for anything that would require installation before configuring. (we also have install-foo for anything that just installs without any configuration or removal steps)

shortcuts like ujust benchmark would stay as they are however i can merge all the ujust zsh|fish|bash into ujust configure-shell with an alias named ujust shell

Other than this there does not seem to be anything else to do other than just some minor cleanup and change recipes to follow the standard we decided on in ublue-os/config#181

@HikariKnight HikariKnight self-assigned this Mar 4, 2024
@m2Giles
Copy link
Member

m2Giles commented Mar 4, 2024

Shells:
I think we have multiple just commands for switching default using various methods. Maybe make this one command and have a selector between them?

Configuration:
We don't have a lot of common verbiage. Most seem to be one direction setups. For one direction I think setup would be good verbiage. Toggles could be configure.

Overall we probably have a lot of things that could be removed. We also should consider not in lining so much stuff.

@castrojo
Copy link
Member

castrojo commented Mar 4, 2024

Maybe make this one command and have a selector between them?

This would be great for shells!

@HikariKnight
Copy link
Member Author

HikariKnight commented Mar 9, 2024

Currently since there have not been any verbs before on anything, i have left in aliases to the old recipe names so that you all can remove them when the time is right for it

But i have updated everything (that made sense to update) to use the verbs we agreed on in the config repo.
i put gnome-vrr as toggle- but i can switch it to configure- if that makes more sense since it is the only toggle in the system (other than devmode, but i feel devmode is important enough to be kept without a verb)

changes gnome-extensions to get-gnome-extensions
changes gnome-vrr to toggle-gnome-vrr
As the containerfile flattens all the files into 1 just file, it is nice to see which part of the file originates from where on the system
@HikariKnight HikariKnight marked this pull request as ready for review March 9, 2024 09:12
@HikariKnight HikariKnight requested a review from castrojo as a code owner March 9, 2024 09:12
@HikariKnight HikariKnight changed the title chore(just): cleanup and standardize just recipes feat(just): cleanup and standardize just recipes Mar 9, 2024
@HikariKnight HikariKnight changed the title feat(just): cleanup and standardize just recipes feat(just): cleanup and standardization of just recipes Mar 9, 2024
Copy link
Member

@m2Giles m2Giles left a comment

Choose a reason for hiding this comment

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

Left a few comments. Overall looks solid

just/bluefin-system.just Outdated Show resolved Hide resolved
Comment on lines +32 to +37
# if /etc/profile.d/brew.sh already exists, replace it with /usr/etc/profile.d/brew.sh
if [ -f /etc/profile.d/brew.sh ]; then
if [ -f /usr/etc/profile.d/brew.sh ]; then
sudo cp /usr/etc/profile.d/brew.sh /etc/profile.d/brew.sh
fi
fi
Copy link
Member

Choose a reason for hiding this comment

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

I get that we may want to do this, but instead of copying over, maybe a symlink might make more sense? This might be a change we need to do on our /etc/profile.d/brew.sh

If that's a file then the user has modified, if it's a symlink we won't have to update? Since this prompt doesn't go away, I would want to avoid overwriting user changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can change that, i think @bketelsen wrote that originally, might be a reason why it is not a symlink instead.
Will wait for them to reply before making that change though, but i am pretty sure a symlink will be fine.

Copy link
Member

Choose a reason for hiding this comment

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

no reason, symlink is a great idea. please proceed!

Copy link
Member

@m2Giles m2Giles left a comment

Choose a reason for hiding this comment

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

We'll wrap up the brew symlink in another PR. LGTM.

@castrojo castrojo merged commit 4b9441a into main Mar 14, 2024
21 of 30 checks passed
@castrojo castrojo deleted the just-reorg branch March 14, 2024 19:52
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.

4 participants