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

chore: Remove usage of yq in favor of jq #369

Merged
merged 19 commits into from
Dec 2, 2024
Merged

Conversation

fiftydinar
Copy link
Collaborator

@fiftydinar fiftydinar commented Dec 1, 2024

@fiftydinar fiftydinar requested a review from xynydev as a code owner December 1, 2024 10:42
github-actions[bot]

This comment was marked as off-topic.

@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

shellcheck

📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

unit=$(printf "$unit")


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

systemctl --global -f enable $unit


📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

unit=$(printf "$unit")


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

systemctl --global disable $unit


📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

unit=$(printf "$unit")


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

systemctl --global unmask $unit


📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

unit=$(printf "$unit")


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

systemctl --global mask $unit


[shellcheck] reported by reviewdog 🐶
$/${} is unnecessary on arithmetic variables. SC2004

INSTALL_PKGS[$i]="${PKG//%OS_VERSION%/${OS_VERSION}}"


⚠️ [shellcheck] reported by reviewdog 🐶
Quote this to prevent word splitting. SC2046

rpm-ostree override remove "${REMOVE_PKGS[@]}" $(printf -- "--install=%s " "${CLASSIC_PKGS[@]}")


⚠️ [shellcheck] reported by reviewdog 🐶
Quote this to prevent word splitting. SC2046

rpm-ostree override remove "${REMOVE_PKGS[@]}" $(printf -- "--install=%s " "${CLASSIC_PKGS[@]}")


⚠️ [shellcheck] reported by reviewdog 🐶
Quote this to prevent word splitting. SC2046

rpm-ostree override remove "${REMOVE_PKGS[@]}" $(printf -- "--install=%s " "${CLASSIC_PKGS[@]}")


⚠️ [shellcheck] reported by reviewdog 🐶
Quote this to prevent word splitting. SC2046

rpm-ostree override remove "${REMOVE_PKGS[@]}" $(printf -- "--install=%s " "${CLASSIC_PKGS[@]}")


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

rpm-ostree override replace --experimental --from "repo=copr:copr.fedorainfracloud.org:${MAINTAINER}:${REPO_NAME}" ${REPLACE_STR}


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086


📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

echo "Adding to $INSTALL_LEVEL flatpak installs: $(printf ${flatpak})"


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo "Adding to $INSTALL_LEVEL flatpak installs: $(printf ${flatpak})"


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo $flatpak >> $INSTALL_LIST


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo $flatpak >> $INSTALL_LIST


📝 [shellcheck] reported by reviewdog 🐶
Don't use variables in the printf format string. Use printf '..%s..' "$foo". SC2059

echo "Adding to $INSTALL_LEVEL flatpak removals: $(printf ${flatpak})"


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo "Adding to $INSTALL_LEVEL flatpak removals: $(printf ${flatpak})"


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo $flatpak >> $REMOVE_LIST


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

echo $flatpak >> $REMOVE_LIST


📝 [shellcheck] reported by reviewdog 🐶
Possible misspelling: FILES may not be assigned. Did you mean FILE? SC2153

if [[ ${#FILES[@]} -gt 0 ]]; then


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

cp -rf "$FILE"/* $DEST


⚠️ [shellcheck] reported by reviewdog 🐶
=~ is for regex, but this looks like a glob. Use = instead. SC2049

if [[ "${DEST}" =~ *"/" ]] || [[ "${DEST}" == "/" ]]; then


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

cp -f $FILE $DEST


📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

cp -f $FILE $DEST


⚠️ [shellcheck] reported by reviewdog 🐶
=~ is for regex, but this looks like a glob. Use = instead. SC2049

if [[ "${DEST}" =~ *"/" ]] || [[ "${DEST}" == "/" ]]; then

@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
@blue-build blue-build deleted a comment from github-actions bot Dec 1, 2024
modules/default-flatpaks/v1/default-flatpaks.sh Outdated Show resolved Hide resolved
modules/files/files.sh Outdated Show resolved Hide resolved
Copy link
Member

@xynydev xynydev left a comment

Choose a reason for hiding this comment

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

Alright, I've combed through and it LGTM.

One concern; does this possible break some people's builds completely? I don't see jq being included by ublue, but maybe it's included in base Fedora? But get_json_array would still break at least the builds on the legacy template, but it's time to retire support for that anyways I guess, it should just come with an announcement. Including the jq addition in some point releases for previous CLI releases could be a possibility, but maybe too overkill.

@xynydev xynydev requested a review from gmpinder December 2, 2024 06:53
@fiftydinar
Copy link
Collaborator Author

One concern; does this possible break some people's builds completely?

Only those in legacy templates & those with custom modules.

I don't see jq being included by ublue, but maybe it's included in base Fedora?

It's in base Fedora.

But get_json_array would still break at least the builds on the legacy template, but it's time to retire support for that anyways I guess, it should just come with an announcement.

That would be good, also announcement for custom module users. They can just install yq or migrate to jq like we did.

Including the jq addition in some point releases for previous CLI releases could be a possibility, but maybe too overkill.

I would say it's overkill imo.

@xynydev
Copy link
Member

xynydev commented Dec 2, 2024

But get_json_array would still break at least the builds on the legacy template, but it's time to retire support for that anyways I guess, it should just come with an announcement.

That would be good, also announcement for custom module users. They can just install yq or migrate to jq like we did.

Yeah, I guess I'll put a TODO for doing an announcement today, and then line all this up for merge. We could announce the CLI updates at the same time too.

@xynydev xynydev merged commit 189048b into main Dec 2, 2024
5 checks passed
@xynydev xynydev deleted the switch-from-yq-to-jq branch December 2, 2024 19:07
@xynydev xynydev mentioned this pull request Dec 4, 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.

refactor: search for yq in the codebase, and replace all occurences with appropriate jq commands
3 participants