Re: [PATCH 1/2] Readme.md: add submodule policy and clone commands

["Wang, Jian J" <jian.j.wang@intel.com>]

Leif,

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Tuesday, July 09, 2019 5:27 PM
> To: Wang, Jian J <jian.j.wang@intel.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [PATCH 1/2] Readme.md: add submodule policy and clone
> commands
> 
> Hi Jian,
> 
> Many thanks for this.
> A few comments on the text.
> 
> On Tue, Jul 09, 2019 at 02:36:00PM +0800, Jian J Wang wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1910
> >
> > A section 'Submodules' is added to clarify the submodule policy
> > in edk2 repo. Git commands are also added to show the correct
> > way to clone submodule repos, in which '--recursive' is removed
> > because it's not needed but recommended in other document.
> >
> > Related commits:
> > Openssl-1.1.1b upgrade: acfb90911840c38a0beb9bcfe0065668244d2b4d
> > berkeley-softfloat-3:   3cc57695df5a6e8c65fb46b993836c315cabf49d
> >
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  Readme.md | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/Readme.md b/Readme.md
> > index e564c6c09b..ddb4da5648 100644
> > --- a/Readme.md
> > +++ b/Readme.md
> > @@ -143,3 +143,22 @@ Signed-off-by: Contributor Name
> <contributor@example.com>
> >    the change.  Each line should be less than ~70 characters.
> >  * `Signed-off-by` is the contributor's signature identifying them
> >    by their real/legal name and their email address.
> > +
> > +# Submodules
> > +
> > +As a general policy, submodules should be avoided in EDK II repo as
> possible as we can, especially submodules required by other submodules.
> Currently EDK II  contains two submodules
> 
> Please wrap lines at 80 characters.
> One of the benefits of markdown is that it can be easily read both as
> a plain text file, and be rendered into something else (like html for
> displaying on the web).
> 

I used markdown editor to do this. I didn't notice the line length. I'll wrap
it.

> > +
> > +- CryptoPkg/Library/OpensslLib/openssl
> > +- ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3
> > +
> > +The later one is actually required by previous one. It's inevitable in
> openssl-1.1.1 (since stable201905) for floating point parameter conversion,
> but should be dropped once there's no such need in future release of
> openssl.
> 
> "later" -> "latter" when referring to items in an enumeration
> (interesting quirk of English)
> 

Thanks for catching this. I might have used this wrong word for years:(

> > +
> > +Note: When cloning submodule repos, '--recursive' option is not
> recommended. EDK II itself will not use any code/feature from submodules
> in above submodules. '--recursive' might cause failure in cloning behind
> proxy.
> 
> I wouldn't say "behind proxy". It may fail because it adds additional
> servers that must be reachable in order for the clone to succeed.
> 
> So we could replace the last sentence above with something like:
> "So using '--recursive' adds a dependency on being able to reach
> servers we do not actually want any code from, as well as needlessly
> downloading code we will not use."
> 
> I would also prefer to move this "Note:" to immediately below the
> command example:

You're right. I'll use your version instead in v2. Thanks for the comments.

Regards,
Jian
> 
> > +
> > +To get a full, buildable EDK II repository, use following two steps of git
> command
> > +
> > +```
> > +$ git clone https://github.com/tianocore/edk2.git
> > +$ git submodule update --init
> > +```
> > +
> 
> ("Note:" here.)
> 
> /
>     Leif
> 
> > --
> > 2.17.1.windows.2
> >