| Message ID | 1355774648.20811.YahooMailNeo@web120305.mail.ne1.yahoo.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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" == 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?
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))
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