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

["Leif Lindholm" <leif.lindholm@linaro.org>]

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

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

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

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