a-barinov / liteqube

Liteqube - put Qubes OS on a diet
54 stars 5 forks source link

Improve overall code quality #22

Open emanruse opened 11 months ago

emanruse commented 11 months ago

This is a very interesting project. Thank you for working on it.

If I may suggest, an improvement in overall code quality would make this good work even better. After all, this is something which one is supposed to run in dom0 and it is good to have the possibility to go through it easily before using it. Personally, it took me more than 2 hours to check /.lib only (and I write bash scripts almost every day).

While there is no unified standard about shell script coding, there are some good practices. To name a few (not a complete list):

  1. Don't use VAR_NAMES_IN_CAPS. Use lowercase_name instead. This avoids the potential problem of conflicting environment variable names (which are uppercase).

  2. Use local variables whenever possible

my_function()
{
    local my_var="${1}"
    ...
}

and you won't need to use cryptic prefixes like _VRP_SIZE.

  1. I think [ x"$a" = x"$b" ] is outdated. The preferred way (especially if you use Bash which is installed in dom0) is [[ "${a}" = "${b}" ]].

  2. If you need to comment what the code does (e.g. # Check if file exists), this means the code is not readable. Commands and variable names should speak for themselves. Comments should explain why something is done, not what is done.

  3. Don't write complex nested loops and ifs (like in .lib/check-with-local.sh). Complex code is not only difficult to read but also difficult to maintain.

  4. Limit line length to 80. It's more readable.

  5. The result of any code execution should be predictable even in unpredictable situations and handled properly.

  6. Exit early.

E.g. this:

my_function()
{
    if [ -e "${some_file}" ]; then
        add_line dom0 "some" "thing"
    else
        add_line dom0 "something" "else"
    fi
}

is better written as:

my_function()
{
    if [ -e "${some_file}" ]; then
        add_line dom0 "some" "thing"
        return
    fi
    add_line dom0 "something" "else"
}
  1. Short lines and vertical orientation are more readable.

This:

command arg1 \
    arg2 \
    arg3 \
    | another_command \
    | third command
fourth_command

is more readable than this:

command arg1 arg2 arg3 | another_command | third command'; fourth_command
  1. Check your scripts with ShellCheck

And so on. There are some good books about general code readability.

Please also add a license, so others can contribute. Keep up the good work!

froeb commented 9 months ago

Great sugestions emanruse. The crator seems to have abandoned this project or at least seems not to reply. Do you have a fork available where you implemented your recomenadations at least for some files/directories so that one can use your improvements?

emanruse commented 9 months ago

Do you have a fork available where you implemented your recomenadations at least for some files/directories so that one can use your improvements?

Forking is a form of copying. There is no license file, hence it is not clear whether copying would be legal, so I am not even considering that. Besides, digging through another's code for the sake of refactoring it is a huge effort. That should generally be done by the author who knows the code better than anyone.