[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 for-4.14] docs/support-matrix: unbreak docs rendering



Andrew Cooper writes ("[PATCH v2 for-4.14] docs/support-matrix: unbreak docs 
rendering"):
> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
> while.  commitish_version() pulls an old version of xen/Makefile out of
> history, and uses the xenversion rule.
> 
> Currently, this fails with:

Thanks for fixing this!

> +    local maj=$(sed -n 's/.*XEN_VERSION.*= \([0-9]\+\)/\1/p' < 
> "$tmp_versionfile")
> +    local min=$(sed -n 's/.*XEN_SUBVERSION.*= \([0-9]\+\)/\1/p' < 
> "$tmp_versionfile")

> +    if [[ -z $maj || -z $min ]];

I would prefer to avoid use of the unusual bash-specific [[ ]] syntax,
not because of concerns about portability to other shells (since this
is a #!/bin/bash script) but because many programmers won't be
familiar with it.

Would you mind writing this instead

  +    if test -z "$maj" || test -z "$min"; then

?

> +    printf "%d.%d\n" "${maj}" "${min}"

The { } here are not necessary but I don't kind if you feel they add
clarity.

You might also want to retain some text in the comment about what
assumptions we are making about xen/Makefile.  Different assumptions
to before, but assumptions nevertheless.

In any case, with or without the changes I suggest above:

Reviewed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

On IRC:

14:35 <andyhhp__> Diziet: jbeulich: For the docs support-matrix
fix, any concerns with me backporting it immediately?

Having thought about this properly, I don't think we need to urgently
backport this.  In our own infrastructure, the one from staging (or
maybe it is master) is used.  The same script is used to parse all
older versions.

But I think it is a backport candidate.  Maybe add a Backport:
tag and my Backport-Requested-By ?

Thanks,
Ian.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.