2017-04-09 21:58 GMT+05:00 Steffan Karger <stef...@karger.me>:
> Hi,
>
> On 09-04-17 15:31, Илья Шипицин wrote:
> > > > @@ -70,7 +84,6 @@ if [ "${TRAVIS_OS_NAME}" != "osx" ]; then
> > > > fi
> > > >
> > > > # Download and build crypto lib
> > > > -mkdir -p download-cache
> > > > if [ "${SSLLIB}" = "openssl" ]; then
> > > > download_openssl
> > > > build_openssl
> > > > @@ -81,3 +94,35 @@ else
> > > > echo "Invalid crypto lib: ${SSLLIB}"
> > > > exit 1
> > > > fi
> > > > +
> > > > +if [ ! -z ${CHOST+xxx} ]; then
> > > > + echo "deb http://archive.ubuntu.com/ubuntu
> > <http://archive.ubuntu.com/ubuntu> <http://archive.ubuntu.com/ubuntu
> > <http://archive.ubuntu.com/ubuntu>>
> > > xenial main universe" | sudo tee -a /etc/apt/sources.list.d/
> xenial.list
> > > > + echo "deb http://archive.ubuntu.com/ubuntu
> > <http://archive.ubuntu.com/ubuntu> <http://archive.ubuntu.com/ubuntu
> > <http://archive.ubuntu.com/ubuntu>>
> > > xenial main" | sudo tee -a /etc/apt/sources.list.d/xenial.list
> > > > + sudo apt-get update
> > > > + sudo apt-get -y install dpkg mingw-w64
> > > > + if [ ! -f
> > "download-cache/tap-windows-${TAP_WINDOWS_VERSION}.zip"
> ]; then
> > > > + wget -P download-cache/ http://build.openvpn.net/
> downloads/releases/tap-windows-${TAP_WINDOWS_VERSION}.zip
> > <http://build.openvpn.net/downloads/releases/tap-
> windows-${TAP_WINDOWS_VERSION}.zip>
> > > <http://build.openvpn.net/downloads/releases/tap-
> windows-${TAP_WINDOWS_VERSION}.zip
> > <http://build.openvpn.net/downloads/releases/tap-
> windows-${TAP_WINDOWS_VERSION}.zip>>
> > > > + fi
> > >
> > > This should be in a download_tap_windows() function.
> > >
> > >
> > > can you be more particular on "should be".
> > > it does not mean "must be".
> > > if so, we can leave it, right ?
> >
> > I'll be more clear (and less polite..): writing long lists of
> commands
> > is bad. That's not only true for code, it goes for scripts as well.
> > Writing functions (with good names!) allows the reader to not have to
> > parse the commands to understand what the author wanted to do.
> >
> > If we put this in functions, the pkcs11 section will read
> >
> > download_pkcs11helper
> > build_pkcs11helper
> >
> > Which makes it immediately clear what the intent of that code is.
> >
> > For tap-windows we only have to download and extract, to that would
> only
> > read
> >
> > download_tap_windows
> >
> > which makes this immediately clear too.
> >
> > This really is basic software engineering, and the lack of quality in
> > quite some of your contributions is an important reason why your
> > contributions aren't picked up quickly. Your ideas and intentions
> are
> > good, but we need to put in far too much effort to make the quality
> of
> > your contribution acceptable. Time that we can also spend on
> > better-quality contributions. It would help us all (including you)
> if
> > you would try to improve on that.
> >
> >
> >
> > those code lines are not reusable, so not splitting it into functions
> > does not mean poor quality (for me)
> > the rule of thumb is "to put into function piece of code, which is
> > called many times"
> >
> > otherwise, it might be either function or not.
>
> Functions are not only to prevent code duplication. They are also for
> structuring your code. Which is exactly what I'm suggesting here. Code
> does not only need to do it's job, it also needs to be maintainable.
>
> > 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 ?
>
> We're not here to give software engineering classes. Nor is the openvpn
> wiki meant to be a book on software engineering.
>
well, I did not ask for software engineering classes.
what I still do not understand is your "ultimate" statement about "poor
code quality".
bash allows either use or not use functions. openvpn does not propose any
standards on that.
so, what I did - I wrote very small piece of code, which fits single
screen, also it is
pretty self documented (wget --> tar --> make). I still think it is good
enough.
yes, we can use functions if people on list would agree on that, but I
think there were not enough reasons for calling that "poor code quality".
>
> -Steffan
>
------------------------------------------------------------------------------
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
