Re: Request for suggestions on a rails-apps-common package

[Per Andersson <avtobiff@gmail.com>]

On Tue, Oct 13, 2015 at 1:17 AM, Per Andersson <avtobiff@gmail.com> wrote:
> On Thu, Sep 24, 2015 at 8:36 AM, Pirate Praveen
> <praveen@onenetbeyond.org> wrote:
>> Hi,
>>
>> Currently diaspora-common has adduser.sh (creates system user),
>> grantpriv.sh (setup database permissions) and set-env-nginx.sh (creates
>> nginx configuration) and is used by diaspora and diaspora-installer
>> packages.
>>
>> Now I need to do exactly same tasks for gitlab. Does it make sense to
>> create a rails-apps-common package? Or is it too small a change that I
>> can just copy the files and adapt?
>
> Will it be exactly similar for gitlab (and other rails apps)? Is it possible
> that there will be a need for small customizations anyway?
>
>
> Some notes on the shell scripting style used
>
> * It is better to use $(...) instead of backtick operator `...` for subshells,
>   a lot better readability.
>
> * Don't mix tabs and spaces in source code files (unless strapped for
>   space, like in embedded systems).
>
> * It is ok to space code blocks. Actually I encourage it. This is also for
>   readability.
>
> * It is good practice to set errexit option (commonly, set -e) in order to
>   stop script execution if something went wrong (exit code !=, broken pipe
>   etc).
>
> * Strings are good to enclose in citation marks, especially if they are
>   user input. E.g. [ "x$1" = "x"] and export SERVERNAME="$1".
>
> * Any reason why $SERVERNAME isn't used in set-env-nginx.sh in
>   favour of $1? (Which needs a full reading of the script to understand
>   what is actually going on.)
>
> * It is possible to use another delimiter char in sed substitions then
>   slash, e.g. s#input#output#. Use it and there is no need to escape
>   slashes.

* Unnecessary to create a new block for only one command e.g.
  something-failing || { exit 1 } . Just omit the braces.

* Create a function for complicated work instead of chaining into
  a block. I.e. replace

    something || { error-handling-spanning-several-lines }

  with

    function error-handling() { ... }
    something || error-handling

* Possibly consider using /usr/sbin/nologin instead of /bin/false.
  (I am unsure of best practice and implications though.)


--
Per