Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2 1/1] jsen: new package JSEN (JSON Sentinel) validates your JSON objects using JSON-Schema.
Date: Wed, 17 Feb 2016 13:07:40 +0100	[thread overview]
Message-ID: <20160217130740.7bb84a20@free-electrons.com> (raw)
In-Reply-To: <1455706383-8951-1-git-send-email-atul.singh.mandla@rockwellcollins.com>

Dear Atul Singh,

Thanks for this new version. I could apply it by doing a few changes,
but I prefer to give you a review so you know what the remaining issues
are.

First the title of the commit should be:

	jsen: new package

and nothing else. If you want to put some additional explanations, you
need to separate it with an empty new line. Like this:

"""
jsen: new package

This new package adds the JSEN Javascript library. JSEN (JSON Sentinel)
validates your JSON objectifs using JSON-Schema.

Signed-off-by: ....
"""

On Wed, 17 Feb 2016 16:23:03 +0530, Atul Singh wrote:
> Changes v1->v2
> -Added the jsen under the javascript menu(as suggested by Thomas).
> -Updated the loop for retrieving the JSEN directories.

It's very good to have a change log, but it should go....

> 
> Signed-off-by: Atul Singh <atul.singh.mandla@rockwellcollins.com>
> ---

... here, i.e after the "---" sign.

This way, the changelog is part of your patch (so I can see it when
reviewing your patch) but it will not be integrated in the Git history
as part of the commit log.

> diff --git a/package/Config.in b/package/Config.in
> index a5b31aa..1e511de 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -985,6 +985,7 @@ menu "External jQuery plugins"
>  endmenu
>  endif
>  	source "package/jsmin/Config.in"
> +	source "package/jsen/Config.in"
>  	source "package/json-javascript/Config.in"

Alphabetic ordering it not correct here.

> new file mode 100644
> index 0000000..f5949aa
> --- /dev/null
> +++ b/package/jsen/jsen.mk
> @@ -0,0 +1,20 @@
> +################################################################################
> +#
> +# jsen
> +#
> +################################################################################
> +
> +JSEN_VERSION = v0.6.0
> +JSEN_SITE = $(call github,bugventure,jsen,$(JSEN_VERSION))
> +JSEN_LICENSE = MIT
> +JSEN_LICENSE_FILES = LICENSE
> +JSEN_DIRECTORIES_LIST = dist lib
> +JSEN_ROOT_FILES_LIST = index.js

Those two variables are useless now.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

      reply	other threads:[~2016-02-17 12:07 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17 10:53 [Buildroot] [PATCH v2 1/1] jsen: new package JSEN (JSON Sentinel) validates your JSON objects using JSON-Schema Atul Singh
2016-02-17 12:07 ` Thomas Petazzoni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160217130740.7bb84a20@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox