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

shellcheck corrections #24

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Conversation

Yarboa
Copy link
Collaborator

@Yarboa Yarboa commented Nov 29, 2023

shellcheck tests are broken
fixing script based on shell check recommendations

sample https://github.com/containers/initoverlayfs/actions/runs/6946556735

scripts/build.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@ericcurtin ericcurtin left a comment

Choose a reason for hiding this comment

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

I think this is ok, just a question, did we test this?

I worry about things like:

| "$decompressor" |

do they work like we intend, with the lack of CI in this project at present, we have to manually test every change until we get proper CI, that executes this stuff e2e.

@ericcurtin
Copy link
Collaborator

Thanks for looking into this @Yarboa I think this is our first green PR!

@Yarboa Yarboa marked this pull request as draft November 30, 2023 10:39
@Yarboa
Copy link
Collaborator Author

Yarboa commented Nov 30, 2023

Moving to draft,
We will merge this one with automation, good point @ericcurtin

shellcheck tests are broken
fixing script based on shell check recommendations

Signed-off-by: Yariv Rachmani <[email protected]>
@Yarboa Yarboa marked this pull request as ready for review December 7, 2023 21:59
@Yarboa
Copy link
Collaborator Author

Yarboa commented Dec 7, 2023

@ericcurtin
PR was tested with f39 release

@ericcurtin
Copy link
Collaborator

Thanks for testing, mergy mergy

@ericcurtin ericcurtin merged commit 67f2ca8 into containers:main Dec 7, 2023
2 checks passed
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.

2 participants