Re: [PATCH v4] Improve Windows build documentation

[Martin Bonner <martingreybeard@gmail.com>]

Martin


On Wed, 27 Jul 2022 at 15:15, Tom Rini <trini@konsulko.com> wrote:

> On Wed, Jul 27, 2022 at 03:56:07PM +0200, Heinrich Schuchardt wrote:
> > On 7/27/22 15:51, Heinrich Schuchardt wrote:
> > > On 7/27/22 15:29, Heinrich Schuchardt wrote:
> > > > On 7/25/22 09:42, Martin Bonner wrote:
> > > > > * Add three more modules that are required.
> > > > > * Remove the version numbers (because they are hard to keep in sync
> > > > >    with the latest MSYS2 versions)
> > > > > * Add a pacman command line to install everything.
> > > > >
> > > > > Signed-off-by: Martin Bonner <martingreybeard@gmail.com>
> > > >
> > > > Your mail is not a valid patch:
> > > >
> > > > $ git am /tmp/0.patch
> > > > Applying: Improve Windows build documentation
> > > > error: corrupt patch at line 10
> > > > Patch failed at 0001 Improve Windows build documentation
> > > >
> > > > Please, use 'git send-email' for sending patches.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > > ---
> > > > >   doc/build/tools.rst | 22 ++++++++++++++--------
> > > > >   1 file changed, 14 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/doc/build/tools.rst b/doc/build/tools.rst
> > > > > index c06f915274..5f8a04a31b 100644
> > > > > --- a/doc/build/tools.rst
> > > > > +++ b/doc/build/tools.rst
> > > > > @@ -24,14 +24,20 @@ you can use MSYS2, a software distro and
> building
> > > > > platform for Windows.
> > > > >   Download the MSYS2 installer from https://www.msys2.org. Make
> sure
> > > > > you have
> > > > >   installed all required packages below in order to build these
> host
> > > > > tools::
> > > > >
> > > > > -   * gcc (9.1.0)
> > > > > -   * make (4.2.1)
> > > > > -   * bison (3.4.2)
> > > > > -   * diffutils (3.7)
> > > > > -   * openssl-devel (1.1.1.d)
> > > > > -
> > > > > -Note the version numbers in these parentheses above are the
> package
> > > > > versions
> > > > > -at the time being when writing this document. The MSYS2 installer
> > > > > tested is
> > > > > +   * gcc
> > > > > +   * make
> > > > > +   * bison
> > > > > +   * diffutils
> > > > > +   * openssl-devel
> > > > > +   * flex
> > > > > +   * libgnutls-devel
> > > > > +   * libuuid-devel
> > >
> > > This should not be pre-formatted text but simply a list.
>

I don't understand what you mean.  That _is_ a (bulleted) list (or have I
misunderstood? - very possible)

> > >
> > > > > +
> > > > > +You probably want ``git`` as well.  You can install all these
> with::
> > >
> > > Allow for syntax highlighting:
> > >
> > > .. code-block:: bash
>
There's no syntax to highlight!

> > >
> > > > > +
> > > > > +    $ pacman -S gcc make bison diffutils openssl-devel flex
> > > > > libgnutls-devel libuuid-devel git
> > >
> > > The line should be limited to 80 characters. Use \.
>
The line length is about 94 characters.  I don't think that is excessive.
I believe Tom has said u-boot has given up on a strict 80-character limit.

> > Please, remove the leading $.
>

I find it helpful to indicate the text is a command.


> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > > > +
> > > > > +The MSYS2 installer tested is
> > > > >   http://repo.msys2.org/distrib/x86_64/msys2-x86_64-20190524.exe.
> >
> > Do you really suggest to download a 3 year old version?
>
> It should be kept in-sync with what .azure-pipeline.yml does which is a
> newer version than that, yes.
>
I can't find a file of that name in the repo.  Wouldn't it be better to
just do as Heinrich suggests, and drop the line entirely?