Re: [PATCH v2] ver_linux: add comparison with required version and add rows and colour to output

Jonathan Corbet <[email protected]>
Newsgroups org.kernel.vger.workflows,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Manuel Ebner <[email protected]> writes:

> Add the required version from changes.rst.
> Add code for comparison and print '###' in the respective row (good, bad) and in
> the respective colour (blue, green, red) depending on the result of the comparison.
>
> Signed-off-by: Manuel Ebner <[email protected]>
> ---
> [v2]
> add colour reset as Geert suggested
> change output to take colourblindness into account

So that suggests you read my previous comments, but didn't see fit to
look at all of them or respond.

*How* are you taking colorblindness into account?  Which type of
colorblindness have you tried to address.  Certainly not red/green
which, I believe, is the most common form.

But, more to the point:

- You do not know that your output is going to a terminal at all, much
  less one that understands these escape sequences.

- You do not know whether that terminal, if it exists, is in dark mode
  or not.

- You don't check NO_COLOR to see whether the user has explicitly asked
  to not have colorized output.

This is a simple script meant to check dependencies, it really does not
need this kind of silliness.  Surely there is something more useful you
could work on?

jon
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.