Re: [Minios-devel] [UNIKRAFT PATCH v2 doc_edit] users.rst: clean up ambiguities related to path set-up

[Lars Kurth <lars.kurth.xen@xxxxxxxxx>]

Ray, 

thanks for the patch: sorry for being picky.
Looking forward to v3

Lars

> On 19 Feb 2018, at 19:08, Ray LI <ray4opensource@xxxxxxxxx> wrote:
> 
> After I tested user's guide successfully, I have made sure the 
> ``unikraft-libs`` is just for external libraries.

The commit message should say something like:
"Documented that UK_LIBS is not needed unless an external library is used"

Explanation: Put yourself into the shoes of someone who reads the git comment 
in 3 years. 
That person only needs to know the ghist of the change. So try to be short and 
concise.
The same applies to comments in code in general.

Testing is expected, so stating that you did it distracts. 

> Signed-off-by: Ray LI <ray4opensource@xxxxxxxxx>
> 
> ---
> Changed since v1:
>       * fix understanding error
>       * make sure it is correct according to run app successfully

Thanks for adding the version bit

Personally I prefer following a pattern such as <Verb past tense><what you 
did><and why, if applicable>
E.g.
* Fixed error in v1 
* Tested documentation by building and running the HelloWorld app

Explanation: This makes change logs much easier to read
Remember: some maintainers spend several hours reading other peoples patches

> ---
> doc/guides/users.rst | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/doc/guides/users.rst b/doc/guides/users.rst
> index d0f2d4f..2d347ce 100644
> --- a/doc/guides/users.rst
> +++ b/doc/guides/users.rst
> @@ -20,7 +20,7 @@ Unikraft app has its own repo): ::
> 
> Now edit the Makefile in the app directory. In particular, set the
> ``UK_ROOT`` and ``UK_LIBS`` variables to point to the directories where you
> -cloned the repos above. For instance, assuming the following directory
> +cloned the repos above. Noted that if you don't use external libraries, you 
> don't need to edit ``UK_LIBS`` variable. It is just for external 
> libraries.For instance, assuming the following directory

Typo: "Noted that" => "Note that"
Grammar: "edit ``UK_LIBS`` variable" => edit the ``UK_LIBS`` variable
You need to add a space between: "libraries.For"

Also you may want to add a line break: Lines should be less than 80 characters 
in length. 
Explanation: That is because some people use vim, emacs, etc. 

> structure ::
> 
>   ├── unikraft
> @@ -38,7 +38,6 @@ those variables as follows: ::
> 
>   UK_ROOT ?= $(PWD)/../../unikraft
>   UK_LIBS ?= $(PWD)/../../unikraft-libs
> -Note that the above ``unikraft-libs`` directory is the ``lib`` directory in 
> the ``unikraft`` directory.

OK
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel