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

clean-up: move all shell script code to a function and use a "main" #740

Open
marc-hb opened this issue Jul 14, 2021 · 0 comments
Open

clean-up: move all shell script code to a function and use a "main" #740

marc-hb opened this issue Jul 14, 2021 · 0 comments
Labels
good first issue Good for newcomers P3 Low-impact bugs or features type:discussion Open ended discussion topic type:enhancement New framework feature or request

Comments

@marc-hb
Copy link
Collaborator

marc-hb commented Jul 14, 2021

This a very generic, clean-up task. Its main purpose is to document this good practice: almost all the code in a shell script should be in a function. In other words, the smallest script should look like this:

main()
{
  do stuff
}

main "$@"

Rationales:

  • More variables can be local. This reduces the risk of accidentally overwriting some user environment variables (side effects)
  • Functions can be defined in any order, they don't need to be defined before being used
  • It provides more line numbers in the FUNCNAME stack on failure
  • moving code around functions does not change the indentation level which helps with git diff, rebase, cherry-pick, merge...
  • it's very easy to comment out the last, main "$@" line and source the entire file without running it to test individual functions interactively (almost like the usual Python's __main__ trick)
  • Similarly, it's very easy to copy/paste functions into an interactive shell and test them.
  • It's easy to temporarily turn off an entire function call for local testing purposes by commenting out a single line.
  • For the same reason, better testability, example: https://opensource.com/article/19/2/testing-bash-bats if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then run_main; fi
  • https://en.wikipedia.org/wiki/Margin_(typography)#The_Digital_Page "Although margin-less web pages do still exist, today it is generally understood that having wide enough margins to provide adequate white space around text is important to the usability and readability of digital text" (unverified)

And last but not least, stating the obvious:

  • A sequence of calls to well named functions is much more readable than a giant and anonymous concatenation of all these functions.

cc:

@marc-hb marc-hb added P3 Low-impact bugs or features good first issue Good for newcomers type:discussion Open ended discussion topic type:enhancement New framework feature or request labels Jul 14, 2021
marc-hb added a commit to marc-hb/sof that referenced this issue Jul 25, 2021
Adds --error-exitcode=1 to valgrind options (otherwise what's the point
of using valgrind?)

Skip alloc test that does not pass on HOST (passes with xt-run)

Add help message.

Runnable from anywhere.

Use shell functions
thesofproject/sof-test#740

Fix all quoting issues and other shellcheck warnings.

Add comments.

Signed-off-by: Marc Herbert <[email protected]>
marc-hb added a commit to marc-hb/sof that referenced this issue Jul 25, 2021
Adds --error-exitcode=1 to valgrind options (otherwise what's the point
of using valgrind?)

Skip alloc test that does not pass on HOST (passes with xt-run)

Add help message.

Runnable from anywhere.

Use shell functions
thesofproject/sof-test#740

Fix all quoting issues and other shellcheck warnings.

Add comments.

Signed-off-by: Marc Herbert <[email protected]>
lgirdwood pushed a commit to thesofproject/sof that referenced this issue Jul 26, 2021
Adds --error-exitcode=1 to valgrind options (otherwise what's the point
of using valgrind?)

Skip alloc test that does not pass on HOST (passes with xt-run)

Add help message.

Runnable from anywhere.

Use shell functions
thesofproject/sof-test#740

Fix all quoting issues and other shellcheck warnings.

Add comments.

Signed-off-by: Marc Herbert <[email protected]>
marc-hb added a commit to marc-hb/sof-test that referenced this issue Oct 15, 2021
marc-hb added a commit that referenced this issue Oct 18, 2021
@marc-hb marc-hb changed the title clean-up: move all shell scripts code to a function clean-up: move all shell script code to a function Jan 11, 2022
marc-hb added a commit to marc-hb/sof-test that referenced this issue Jun 10, 2022
Per rationale detailed in thesofproject#740.

Also add new $TOPDIR constant.

Absolutely zero functional change.

Signed-off-by: Marc Herbert <[email protected]>
marc-hb added a commit that referenced this issue Jun 17, 2022
Per rationale detailed in #740.

Also add new $TOPDIR constant.

Absolutely zero functional change.

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb marc-hb changed the title clean-up: move all shell script code to a function clean-up: move all shell script code to a function and use a "main" Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers P3 Low-impact bugs or features type:discussion Open ended discussion topic type:enhancement New framework feature or request
Projects
None yet
Development

No branches or pull requests

1 participant