Patchwork [1/1] Added new package ncdu

login
register
mail settings
Submitter Bogdan Radulescu
Date Dec. 17, 2012, 8:04 p.m.
Message ID <1355774648.20811.YahooMailNeo@web120305.mail.ne1.yahoo.com>
Download mbox | patch
Permalink /patch/206994/
State Changes Requested
Headers show

Comments

Bogdan Radulescu - Dec. 17, 2012, 8:04 p.m.
Signed-off-by: Bogdan Radulescu <bogdan@nimblex.net>
---
 package/Config.in      |  1 +
 package/ncdu/Config.in |  7 +++++++
 package/ncdu/ncdu.mk   | 13 +++++++++++++
 3 files changed, 21 insertions(+)
 create mode 100644 package/ncdu/Config.in
 create mode 100644 package/ncdu/ncdu.mk
Arnout Vandecappelle - Dec. 17, 2012, 10:34 p.m.
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))
Peter Korsgaard - Jan. 14, 2013, 12:59 p.m.
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi Bogdan,

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

 Arnout>  Some additional comments below.

Could you please send an updated patch taking Arnout's comment into
consideration?

Patch

diff --git a/package/Config.in b/package/Config.in
index 171db8f..2d12cdc 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -732,6 +732,7 @@  if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
 source "package/module-init-tools/Config.in"
 endif
 source "package/monit/Config.in"
+source "package/ncdu/Config.in"
 if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
 source "package/procps/Config.in"
 source "package/psmisc/Config.in"
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"
+       select BR2_PACKAGE_NCURSES
+       help
+         ncdu is a disk usage analyzer with an ncurses interface
+
+         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
+NCDU_SITE=http://dev.yorhel.nl/download/
+NCDU_INSTALL_STAGING_OPT = instroot=$(STAGING_DIR) install
+NCDU_INSTALL_TARGET_OPT = instroot=$(TARGET_DIR) install
+
+NCDU_DEPENDENCIES = ncurses
+
+$(eval $(autotools-package))