Message ID | 508EFA6D.7000404@gmail.com |
---|---|
State | Superseded |
Headers | show |
On 10/29/12 22:51, Zoltan Gyarmati wrote: > Support for the Grantlee library This sentence is redundant. > Grantlee is a Qt implementation of the Django template system, see here: http://www.gitorious.org/grantlee/pages/Home Word-wrap the commit message at +- 75 characters. But actually, this part of the commit message is redundant because it's already in the help text. > > (it's my first patch to buildroot, so please let me know any notes, thx) Doing that :-) Something like this (a note that shouldn't be part of the final commit message) should go below your SOB, separated by --- on a line by itself. Then git-am will strip it off while applying the patch. > > Signed-off-by: "Zoltan Gyarmati" <mr.zoltan.gyarmati@gmail.com> [snip] > diff --git a/package/grantlee/Config.in b/package/grantlee/Config.in > new file mode 100644 > index 0000000..6e3b80d > --- /dev/null > +++ b/package/grantlee/Config.in > @@ -0,0 +1,8 @@ > +config BR2_PACKAGE_GRANTLEE > + bool "grantlee" Config.in indentation should be one tab. > + depends on BR2_PACKAGE_QT This is redundant, because it's already inside an if BR2_PACKAGE_QT (in package/Config.in). > + select BR2_PACKAGE_QT_STL It looks like BR2_PACKAGE_QT_SCRIPT and BR2_PACKAGE_QT_GUI_MODULE are also required. I didn't continue testing, so maybe even that is not enough. > + help > + Qt implemantation of the Django template framework > + > + http://www.grantlee.org/apidox And the help text indentation: one tab + 2 spaces. Also I think http://grantlee.org is a more appropriate URL, even if it just redirects to gitorious. > diff --git a/package/grantlee/grantlee.mk b/package/grantlee/grantlee.mk > new file mode 100644 > index 0000000..9264f80 > --- /dev/null > +++ b/package/grantlee/grantlee.mk > @@ -0,0 +1,14 @@ > +############################################################# > +# > +# grantlee > +# > +############################################################# > + > +GRANTLEE_VERSION = 0.2.0 > +GRANTLEE_SITE = http://downloads.grantlee.org/ Trailing / is redundant. > +GRANTLEE_INSTALL_STAGING = YES > +GRANTLEE_INSTALL_TARGET = YES INSTALL_TARGET defaults to YES, so can be removed. > +GRANTLEE_DEPENDENCIES = host-pkg-config qt pkgconf just got committed, so should be host-pkgconf. But AFAICS it isn't using pkg-config for anything. Also, your mailer messed up the whitespace in your patch. Use git-send-email, that works well. The man page explains how to use it with gmail. Regards, Arnout > + > +$(eval $(cmake-package)) > +
Hi, Thx for the detailed comments, i'll resend it. And as i realized in the debian/ubuntu repos the package called libgrantlee, so maybe it would be better to call it so also in buildroot, as it's more logical name for a library :) What do you think? On 10/31/2012 12:58 AM, Arnout Vandecappelle wrote: > On 10/29/12 22:51, Zoltan Gyarmati wrote: >> Support for the Grantlee library > > This sentence is redundant. > >> Grantlee is a Qt implementation of the Django template system, see >> here: http://www.gitorious.org/grantlee/pages/Home > > Word-wrap the commit message at +- 75 characters. But actually, this > part of the commit message is redundant because it's already in the > help text. > >> >> (it's my first patch to buildroot, so please let me know any notes, thx) > > Doing that :-) > > Something like this (a note that shouldn't be part of the final > commit message) > should go below your SOB, separated by --- on a line by itself. Then > git-am will strip it off while applying the patch. > >> >> Signed-off-by: "Zoltan Gyarmati" <mr.zoltan.gyarmati@gmail.com> > [snip] >> diff --git a/package/grantlee/Config.in b/package/grantlee/Config.in >> new file mode 100644 >> index 0000000..6e3b80d >> --- /dev/null >> +++ b/package/grantlee/Config.in >> @@ -0,0 +1,8 @@ >> +config BR2_PACKAGE_GRANTLEE >> + bool "grantlee" > > Config.in indentation should be one tab. > >> + depends on BR2_PACKAGE_QT > > This is redundant, because it's already inside an > if BR2_PACKAGE_QT (in package/Config.in). > >> + select BR2_PACKAGE_QT_STL > > It looks like BR2_PACKAGE_QT_SCRIPT and BR2_PACKAGE_QT_GUI_MODULE > are also required. I didn't continue testing, so maybe even that > is not enough. > > >> + help >> + Qt implemantation of the Django template framework >> + >> + http://www.grantlee.org/apidox > > And the help text indentation: one tab + 2 spaces. > > Also I think http://grantlee.org is a more appropriate URL, even > if it just redirects to gitorious. > >> diff --git a/package/grantlee/grantlee.mk b/package/grantlee/grantlee.mk >> new file mode 100644 >> index 0000000..9264f80 >> --- /dev/null >> +++ b/package/grantlee/grantlee.mk >> @@ -0,0 +1,14 @@ >> +############################################################# >> +# >> +# grantlee >> +# >> +############################################################# >> + >> +GRANTLEE_VERSION = 0.2.0 >> +GRANTLEE_SITE = http://downloads.grantlee.org/ > > Trailing / is redundant. > >> +GRANTLEE_INSTALL_STAGING = YES >> +GRANTLEE_INSTALL_TARGET = YES > > INSTALL_TARGET defaults to YES, so can be removed. > >> +GRANTLEE_DEPENDENCIES = host-pkg-config qt > > pkgconf just got committed, so should be host-pkgconf. But AFAICS it > isn't > using pkg-config for anything. > > > Also, your mailer messed up the whitespace in your patch. Use > git-send-email, that works well. The man page explains how to use it > with > gmail. > > Regards, > Arnout > > >> + >> +$(eval $(cmake-package)) >> + >
On 10/31/12 09:48, Zoltan Gyarmati wrote: > Hi, > > Thx for the detailed comments, i'll resend it. And as i realized in the debian/ubuntu repos the package called libgrantlee, > so maybe it would be better to call it so also in buildroot, as it's more logical name for a library :) What do you think? In buildroot, we prefer to use the upstream name. In Debian/Fedora distros, they split packages into a lib- and a bin- package (and additional -dev and -dbg packages). For grantlee, there is no bin-, so you don't get a plain grantlee package. In buildroot, we don't split the packages (sometimes there are sub-options to disable the binaries). Therefore, there is no reason to change the name. Regards, Arnout
On 10/31/2012 09:51 AM, Arnout Vandecappelle wrote: > On 10/31/12 09:48, Zoltan Gyarmati wrote: >> Hi, >> >> Thx for the detailed comments, i'll resend it. And as i realized in >> the debian/ubuntu repos the package called libgrantlee, so maybe it >> would be better to call it so also in buildroot, as it's more >> logical name for a library :) What do you think? > > In buildroot, we prefer to use the upstream name. > > In Debian/Fedora distros, they split packages into a lib- and a bin- > package (and additional -dev and -dbg packages). For grantlee, > there is no bin-, so you don't get a plain grantlee package. > > In buildroot, we don't split the packages (sometimes there are > sub-options to disable the binaries). Therefore, there is no reason > to change the name. > > Regards, Arnout > Hi, ok, i see. Is there any policy/standards document or wikipage somewhere which summarizes this kind of practices? I couldn't find it so far. What i found is mostly technical kind of infos. (which is extremly useful of course :) best regards Zoltan Gyarmati
diff --git a/package/Config.in b/package/Config.in index ab966e0..ef68b37 100644 --- a/package/Config.in +++ b/package/Config.in @@ -139,6 +139,7 @@ source "package/qt/Config.in" if BR2_PACKAGE_QT comment "QT libraries and helper libraries" source "package/qtuio/Config.in" +source "package/grantlee/Config.in" endif source "package/x11r7/Config.in" diff --git a/package/grantlee/Config.in b/package/grantlee/Config.in new file mode 100644 index 0000000..6e3b80d --- /dev/null +++ b/package/grantlee/Config.in @@ -0,0 +1,8 @@ +config BR2_PACKAGE_GRANTLEE + bool "grantlee" + depends on BR2_PACKAGE_QT + select BR2_PACKAGE_QT_STL + help + Qt implemantation of the Django template framework + + http://www.grantlee.org/apidox diff --git a/package/grantlee/grantlee.mk b/package/grantlee/grantlee.mk new file mode 100644 index 0000000..9264f80 --- /dev/null +++ b/package/grantlee/grantlee.mk @@ -0,0 +1,14 @@ +############################################################# +# +# grantlee +# +############################################################# + +GRANTLEE_VERSION = 0.2.0 +GRANTLEE_SITE = http://downloads.grantlee.org/ +GRANTLEE_INSTALL_STAGING = YES +GRANTLEE_INSTALL_TARGET = YES +GRANTLEE_DEPENDENCIES = host-pkg-config qt + +$(eval $(cmake-package)) +
Support for the Grantlee library Grantlee is a Qt implementation of the Django template system, see here: http://www.gitorious.org/grantlee/pages/Home (it's my first patch to buildroot, so please let me know any notes, thx) Signed-off-by: "Zoltan Gyarmati" <mr.zoltan.gyarmati@gmail.com> --- package/Config.in | 1 + package/grantlee/Config.in | 8 ++++++++ package/grantlee/grantlee.mk | 14 ++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 package/grantlee/Config.in create mode 100644 package/grantlee/grantlee.mk