[PATCH v4] Improve Windows build documentation

[Tom Rini]

On Wed, Jul 27, 2022 at 04:15:52PM +0100, Martin Bonner wrote:
> Martin
> 
> 
> On Wed, 27 Jul 2022 at 15:15, Tom Rini <trini at 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 at 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)

It's a matter, I think, of how you need to write the rST so that it
renders nicely.  Leading spaces lead to one way, no leading spaces
(what Heinrich asks for) renders another way.  This, I think, is spelled
out in the Linux kernel docs about writing docs, which we now do link
to.

> > > > > > +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.

These three comments at least I think go together.  Have you done a
"make htmldocs" and reviewed the output?  In a code-block tagged as bash
thing should look nice and then it's a general style guide to not start
a command with '$' when there's no sample output to distinguish it from.
And I could go back and forth on if I think that's right but we're
consistent in our docs, and consistency matters more in this regard I
believe.

> > > > > > +
> > > > > > +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?

That too, along with making sure we have appropriate overall wording to
make sure it's clear MSYS2 is to be used, is a better still idea, yes.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220727/6b6473f1/attachment.sig>