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

make setup.sh completely non-interactive #2201

Merged
merged 6 commits into from Sep 21, 2021

Conversation

georglauterbach
Copy link
Member

@georglauterbach georglauterbach commented Sep 19, 2021

Description

Cleans up setup.sh and makes it possible to use the script in a completely non-interactive way. Added -R flag. Moved ENV variables to the top, removed unnecessary functions and renamed functions.

This PR should be the last one added to the 10.2.0 milestone. This PR is not a breaking change. Fixes a regression for SELinux (used OPTARG where OPT was actually correct).

Follow up on #2179

Type of change

  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@georglauterbach georglauterbach added priority/low area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Sep 19, 2021
@georglauterbach georglauterbach added this to the v10.2.0 milestone Sep 19, 2021
@georglauterbach georglauterbach requested a review from a team September 19, 2021 17:13
@georglauterbach georglauterbach self-assigned this Sep 19, 2021
Copy link
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

Nice improvements! ❤️

setup.sh Outdated Show resolved Hide resolved
setup.sh Outdated Show resolved Hide resolved
setup.sh Outdated Show resolved Hide resolved
georglauterbach and others added 2 commits September 20, 2021 08:08
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
@georglauterbach
Copy link
Member Author

@polarathene I removed one line from the .ecrc file for the EditorConfig lint, but the line with "^test/" will still block the whole test directory. I will take care of that with another PR.

@@ -52,7 +67,16 @@ function _show_local_usage
Allows container access to the bind mount content that is private and
unshared with other containers on a SELinux-enabled host.

${ORANGE}EXIT STATUS${RESET}
${LBLUE}Podman${RESET}
-R
Copy link
Member

Choose a reason for hiding this comment

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

No show-stopper. But I was wondering, why you chose an upper-case 'R'. With the exception of -Z, all other parameters are lower-case.

Copy link
Member Author

Choose a reason for hiding this comment

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

The full story is this: I war originally trying to use --rootless, but since getopts does not care about double-dashed flags, and I did not want to increase the complexity of the script, I figured I should go with a single-letter-option. When I used r - and I did in the beginning - I always found myself reminded of --recursive and I knew some scripts where using an uppercase letter to denote that there is a long version available, so I chose -R (without a long version availabe). Feel free to change this with another small PR before v10.2.0 is released if you think this is incorrect - I'd just like to get this PR merged :)

@georglauterbach georglauterbach merged commit 3b8059f into master Sep 21, 2021
@georglauterbach georglauterbach deleted the non-interactive-setup-script branch September 21, 2021 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scripts kind/improvement Improve an existing feature, configuration file or the documentation priority/low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants