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

Replace multiple if statements with case and OR #295

Merged

Conversation

the-mentor
Copy link
Contributor

Pretty much the title says it all.
It simplifies the code and make it much shorter and easier to add new distros/OSs

Let me know if you have any questions

-DM

Copy link
Contributor

@lucyllewy lucyllewy left a comment

Choose a reason for hiding this comment

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

One fix, otherwise good.

quickget Outdated
IMAGE_TYPE="iso"
fi

case ${OS} in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case ${OS} in
case "${OS}" in

Copy link
Contributor Author

@the-mentor the-mentor Dec 30, 2021

Choose a reason for hiding this comment

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

Thanks for the review @diddledani.
Fixed!

168f2ee

@flexiondotorg flexiondotorg merged commit bb6b4fe into quickemu-project:master Feb 17, 2022
deraffe pushed a commit to deraffe/quickemu that referenced this pull request Apr 11, 2023
* Replace multiple if statements with case and OR

* Fix case statement per review suggestion

* Added default fallback for undefined OSs

Co-authored-by: Martin Wimpress <304639+flexiondotorg@users.noreply.github.com>
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.

3 participants