Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/1] Added new package ncdu
Date: Mon, 17 Dec 2012 23:34:57 +0100	[thread overview]
Message-ID: <50CF9E11.5070808@mind.be> (raw)
In-Reply-To: <1355774648.20811.YahooMailNeo@web120305.mail.ne1.yahoo.com>

  Dear Bogdan,

  Thanks for your patch.

  We prefer generally prefer patches to be sent in-line with the correct 
spacing - using git send-email is the easiest way to send a patch.

  Some additional comments below.

On 17/12/12 21:04, Bogdan Radulescu wrote:
>
> Signed-off-by: Bogdan Radulescu<bogdan@nimblex.net>
[snip]
> diff --git a/package/ncdu/Config.in b/package/ncdu/Config.in
> new file mode 100644
> index 0000000..d8b4db4
> --- /dev/null
> +++ b/package/ncdu/Config.in
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_NCDU
> +       bool "ncdu"

  Indentation in Config.in should be a single TAB.

> +       select BR2_PACKAGE_NCURSES
> +       help
> +         ncdu is a disk usage analyzer with an ncurses interface

  ... except for help which should be one TAB + 2 spaces.

> +
> +         http://dev.yorhel.nl/ncdu
> diff --git a/package/ncdu/ncdu.mk b/package/ncdu/ncdu.mk
> new file mode 100644
> index 0000000..26f3b5e
> --- /dev/null
> +++ b/package/ncdu/ncdu.mk
> @@ -0,0 +1,13 @@
> +#############################################################
> +#
> +# ncdu
> +#
> +#############################################################
> +NCDU_VERSION=1.9

  We prefer spaces around the =

NCDU_VERSION = 1.9

> +NCDU_SITE=http://dev.yorhel.nl/download/
> +NCDU_INSTALL_STAGING_OPT = instroot=$(STAGING_DIR) install

  Since you don't install to staging, this is redundant.

> +NCDU_INSTALL_TARGET_OPT = instroot=$(TARGET_DIR) install

  I don't understand where this comes from... It looks like it should be 
DESTDIR=$(TARGET_DIR), which is the default, so this line is redundant as 
well.

  It would be nice to also add the licensing:

NCDU_LICENSE = MIT
NCDU_LICENSE_FILES = COPYING

(test with 'make legal-info')

  Regards,
  Arnout

> +
> +NCDU_DEPENDENCIES = ncurses
> +
> +$(eval $(autotools-package))

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

  reply	other threads:[~2012-12-17 22:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-17 20:04 [Buildroot] [PATCH 1/1] Added new package ncdu Bogdan Radulescu
2012-12-17 22:34 ` Arnout Vandecappelle [this message]
2013-01-14 12:59   ` Peter Korsgaard

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=50CF9E11.5070808@mind.be \
    --to=arnout@mind.be \
    --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