On 10/04/2017 00:44, David Sommerseth wrote: > On 09/04/17 15:31, Илья Шипицин wrote: >> >> how did you came to conclusion that it means poor code quality? >> is it written somewhere on the https://openvpn.net on how to write bash >> scripts ? > > Steffan have given a good reasoning, and I completely agree with him.
Searching for "why split code into functions?" in DuckDuckGo gave this as the first hit: <http://stackoverflow.com/questions/3107400/what-is-the-golden-rule-for-when-to-split-code-up-into-functions#3107416> Two good reasons from that page: 1) Code reuse (=avoid repeating yourself) 2) Readability Your patch adds a big chunk of code under if [ ! -z ${CHOST+xxx} ]; then It is pretty clear _what_ the lines in the code block are doing, but it is not at all obvious _why_ the code block is there. One has to search it for hints like # Must have something to do with mingw-w64... sudo apt-get -y install dpkg mingw-w64 and # This must be building for Windows... unzip download-cache/tap-windows-${TAP_WINDOWS_VERSION}.zip One could also search for the commit that added that block, and notice the commit message: "travis-ci: add 2 mingw "build only configurations"". Nobody should have to spend more than a few seconds trying to figure out why the code block is there. A simple comment before the block would have improved readability _a lot_, e.g. # Build on mingw-w64 if [ ! -z ${CHOST+xxx} ]; then That said, please do what Steffan suggested and split the block into human-readable logical functions and call those functions as needed. Then we can review the patch again and hopefully merge it as-is. -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock > Further, it is impossible to write a documentation which is 100% perfect > on coding style, good coding practice and related subjects. If you > insist on that, I recommend you to visit http://shop.oreilly.com/ or > Amazon and find some good books about it (the review ratings on those > sites are usually fairly good in this regard). So there are no reasons > for us to try to duplicate the work of other writers. > > I would even dare to claim that programming isn't just a science with > only one way to do things properly. But it is a science which requires > you to think thoroughly of what you try to solve, how to solve it and > how to make it easy and understandable for others. In fact, the real > science part of programming (to solve a real and concrete issue) is just > one little part of the whole picture. And this makes me think of Reuven > Lerner and his weekly newsletter. He pulls up a lot of these things > there too, perhaps consider to sign up for that too can be a good idea? > <http://lerner.co.il/newsletter/> >
0x40864578.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
