Update CODINGSTYLE.md

Spotted some minor typos, improved some wordings

CODINGSTYLE.md

@@ -6,7 +6,7 @@ and thus the quality of Alpine Linux, we kindly ask to follow these
 recommendations.
 
 ## Language
-Alpine Linux APKBUILD files are inherently just POSIX shell scripts. Try avoid
+Alpine Linux APKBUILD files are inherently just POSIX shell scripts. Please avoid
 extensions, even if they work or are accepted by busybox ash. (using keyword
 `local` is an exception)
 
@@ -14,8 +14,8 @@ extensions, even if they work or are accepted by busybox ash. (using keyword
 Use snake_case. Functions, variables etc. should be lower-cased with
 underscores to separate words.
 
-Local 'private' variables and functions n global scope should be pre-fixed
-with a single underscore to avoid nameclash with internal variables in
+Local 'private' variables and functions in global scope should be prefixed
+with a single underscore to avoid name clashing with internal variables in
 `abuild`.
 
 Double underscores are reserved and should not be used.
@@ -24,7 +24,7 @@ _my_variable="data"
 ```
 
 ### Bracing
-Curly braces for functions are on the same line.
+Curly braces for functions should be on the same line.
 
 ```sh
 prepare() {
@@ -32,7 +32,7 @@ prepare() {
 }
 ```
 
-Markers to indicate a compound statement, are on the same line.
+Markers to indicate a compound statement should be on the same line.
 
 
 #### if ...; then
@@ -52,7 +52,7 @@ Markers to indicate a compound statement, are on the same line.
 
 #### for ...; do
 ```sh
-	for ...; do
+	for x in foo bar baz; do
 		...
 	done
 ```
@@ -60,13 +60,13 @@ Markers to indicate a compound statement, are on the same line.
 ### Spacing
 All keywords and operators are separated by a space.
 
-For cleanliness sake, make sure there is however never any trailing whitespace.
+For cleanliness sake, make sure there are no trailing whitespaces.
 
 ## Identation
-Indentation is one tab character, alignment is done using spaces. For example
-using the >------- characters to indicate a tab:
+Indentation is one tab character per level, alignment is done using spaces.
+For example (using the >------- characters to indicate a tab):
 ```sh
-prepare()
+build()
 {
 >-------make DESTDIR="${pkgdir}" \
 >-------     PREFIX="/usr"
@@ -99,7 +99,7 @@ foo_bar="$foo"
 ```
 
 ## Subshell usage
-Use `$()` syntax, not backticks, as backticks are hard to spot.
+Use `$()` syntax, not backticks, as backticks are harder to spot.
 
 ## Sorting
 Some items tend to benefit from being sorted. A list of sources, dependencies
@@ -107,5 +107,5 @@ etc. Always try to find a reasonable sort order, where alphabetically tends to
 be the most useful one.
 
 ## Eval
-`eval` is evil and should be avoided.
+`eval` is evil and must not be used.
 

[Natanael Copa]

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.

[tcely]

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.

[Simon F]

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.

[Natanael Copa]

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.

[Simon F]

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.

[tcely]

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.

[Olliver Schinagl]

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'.

[Natanael Copa]

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.