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.lindh...@linaro.org>
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Signed-off-by: Jian J Wang <jian.j.w...@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 <contribu...@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
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43432): https://edk2.groups.io/g/devel/message/43432
Mute This Topic: https://groups.io/mt/32402918/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to