Update CODINGSTYLE.md

Spotted some minor typos, improved some wordings

There may be corner cases where eval may be acceptable?

In any case, I think we should fix the once we have before we make it a hard requirement.

There are cases, admittedly very few, where eval is the safest feature to use.

The objection to anything using a sub-shell or executing a command in the global context does tend to eliminate potential uses for eval already.

Although this wording is quite strong, exceptions can and should always be made when appropriate. But mentioning that an exception is possible under very special cases leads to thinking into the wrong direction. If one thinks that it is forbidden, he tries to find other solutions.

Currently the following packages use eval (find -name APKBUILD | xargs grep -l "eval " | sort):

./community/chromium/APKBUILD
./community/elasticsearch/APKBUILD
./community/libreoffice/APKBUILD
./community/lua-cqueues-pushy/APKBUILD
./community/lua-hiredis/APKBUILD
./community/lua-lgi/APKBUILD
./community/lua-resty-dns/APKBUILD
./community/php7/APKBUILD
./community/roundcubemail/APKBUILD
./community/sane/APKBUILD
./main/acf-lib/APKBUILD
./main/aconf/APKBUILD
./main/dahdi-tools/APKBUILD
./main/fetchmail/APKBUILD
./main/haserl/APKBUILD
./main/kamailio/APKBUILD
./main/lua-alt-getopt/APKBUILD
./main/lua-discount/APKBUILD
./main/lua-expat/APKBUILD
./main/lua-iconv/APKBUILD
./main/lua-ldbus/APKBUILD
./main/lua-lzlib/APKBUILD
./main/lua-maxminddb/APKBUILD
./main/lua-md5/APKBUILD
./main/lua-microlight/APKBUILD
./main/lua-mqtt-publish/APKBUILD
./main/lua-openrc/APKBUILD
./main/lua-pc/APKBUILD
./main/lua-posix/APKBUILD
./main/lua-posixtz/APKBUILD
./main/lua-rex/APKBUILD
./main/lua-sec/APKBUILD
./main/lua-socket/APKBUILD
./main/lua-stdlib/APKBUILD
./main/lua-struct/APKBUILD
./main/lua-subprocess/APKBUILD
./main/lua-xctrl/APKBUILD
./main/musl/APKBUILD
./main/nagios-plugins/APKBUILD
./main/nginx/APKBUILD
./main/openssh/APKBUILD
./main/perdition/APKBUILD
./main/sircbot/APKBUILD
./main/u-boot/APKBUILD
./main/uwsgi/APKBUILD
./testing/lua-middleclass/APKBUILD
./testing/openresty/APKBUILD
./testing/php7-diseval/APKBUILD
./testing/proftpd/APKBUILD
./testing/py-serpent/APKBUILD

Some of them (quickly checked openssh and nginx) could be made "eval-free" without too great efforts.

exceptions can and should always be made when appropriate.

This is my point. But if we write "must not" we will have people start arguing about those claiming the text "must not" means "no exceptions". And if we do allow exceptions, then people will start expect exceptions for other things that says "must not" and we have redefined "must not" to actually mean "should not".

Some of them (quickly checked openssh and nginx) could be made "eval-free" without too great efforts.

We should fix those regardless of what the text says. But that's out of scope for this MR.

That's right, we must not redefine the meanings of "must not" ;)

Ok, what do you say to the following part:

`eval` is evil and can lead to unreadable code, security issues and other undesired side effets.
Therefore it should be avoided unless there is no other sane way to achieve the same result.
Nevertheless, any use of `eval` must have a comment nearby to explain why you have chosen to use it there.

I added a few touch ups to your proposal.

eval is evil. Using it can lead to unreadable code, security issues and other undesired side effects. Therefore, it should be avoided unless there is no better way to achieve the same result. Any intentional use of eval must have a comment nearby to explain why you have chosen to use it.

changed this line in version 3 of the diff

Wording matters though; using 'must not' while trying to push people to do things without it, may still have an exception or two. Which is fine, but when a review comes a long, that doesn't know this, they will reject the MR quoting the 'it says must not'.

I don't think we need explain why its evil and we don't explain why we have chosen the other things either. So I think current wording is better.

eval is evil and should be avoided.

Its short, it is simple and opens for exceptions.

added 1 commit

• 107a58b9 - Apply suggestion to CODINGSTYLE.md

Compare with previous version

added 1 commit

• 74977a65 - Update CODINGSTYLE.md

Compare with previous version

added 1 commit

• 08686f1c - Apply suggestion to CODINGSTYLE.md

Compare with previous version

added 1 commit

• 18f079ae - Apply suggestion to CODINGSTYLE.md

Compare with previous version

I also prefer to have vertical style lists split by a newline. Having longer lists its much easier to analyze diffs when a single item changes. I matches the alfa sorting which is already mentioned.

Lets do that separate

I removed the last hunk and squashed the commits.

Fixed with 36151e1e

closed