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

add toolchain build/push and install scripts #63

Merged
merged 5 commits into from
Apr 9, 2024

Conversation

filariow
Copy link
Member

@filariow filariow commented Apr 8, 2024

Signed-off-by: Francesco Ilario filario@redhat.com

Copy link
Member

@sadlerap sadlerap left a comment

Choose a reason for hiding this comment

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

Had a bunch of suggestions from running shellcheck locally, not sure why CI didn't pick up on them. Let me look into it.

hack/toolchain_build_push.sh Outdated Show resolved Hide resolved
hack/toolchain_build_push.sh Outdated Show resolved Hide resolved
hack/toolchain_build_push.sh Outdated Show resolved Hide resolved
hack/toolchain_install.sh Outdated Show resolved Hide resolved
hack/toolchain_install.sh Outdated Show resolved Hide resolved
hack/toolchain_install.sh Show resolved Hide resolved
hack/toolchain_install.sh Outdated Show resolved Hide resolved
hack/toolchain_install.sh Outdated Show resolved Hide resolved
@filariow
Copy link
Member Author

filariow commented Apr 9, 2024

Had a bunch of suggestions from running shellcheck locally, not sure why CI didn't pick up on them. Let me look into it.

On my local, shellcheck v0.9.0 is not providing any suggestion. Which version of shellcheck are you using?
I think the check that suggests changes like $f -> ${f} is an optional one. Did you enable some optional checks?

EDIT: I can see the -o all flag. I added the .shellcheckrc file in #66. WDYT?

filariow added 2 commits April 9, 2024 10:07
Signed-off-by: Francesco Ilario <filario@redhat.com>
Signed-off-by: Francesco Ilario <filario@redhat.com>
filariow added 2 commits April 9, 2024 10:19
Signed-off-by: Francesco Ilario <filario@redhat.com>
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow filariow requested a review from sadlerap April 9, 2024 08:38
@sadlerap
Copy link
Member

sadlerap commented Apr 9, 2024

Yes, I've largely been running it locally with the -o all flag (local version is 0.9.0). It might be a bit overkill though...

@filariow
Copy link
Member Author

filariow commented Apr 9, 2024

Yes, I've largely been running it locally with the -o all flag (local version is 0.9.0). It might be a bit overkill though...

I think those extra checks are good to have. As #66 is merged, we'll have them by default without needing the -o all 😄

@sadlerap
Copy link
Member

sadlerap commented Apr 9, 2024

I'll go take out -o all from .github/workflows/security.yaml, since I explicitly set it there. Now that we have a .shellcheckrc, it should be safe to remove.

@sadlerap
Copy link
Member

sadlerap commented Apr 9, 2024

Filed #67.

@filariow filariow merged commit 0b2ce4f into konflux-workspaces:main Apr 9, 2024
7 checks passed
@filariow filariow deleted the split-scripts branch April 9, 2024 14:54
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.

2 participants