From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] new variable <pkg>_CONFIG_FIXUP
Date: Mon, 7 Jan 2013 23:10:37 +0100 [thread overview]
Message-ID: <20130107231037.39a798cf@skate> (raw)
In-Reply-To: <1357596078-7762-2-git-send-email-stefan.froberg@petroprogram.com>
Dear Stefan Fr?berg,
On Tue, 8 Jan 2013 00:01:18 +0200, Stefan Fr?berg wrote:
> Signed-off-by: Stefan Fr?berg <stefan.froberg@petroprogram.com>
Please add more details to your commit log. I.e, basically, most of your
introduction e-mail contents should go here.
Remember: the contents of your introduction e-mail are not preserved in
the Git history. On the opposite, the commit log is preserved forever
in the Git history, so we want the details to be here.
> ---
> package/pkg-generic.mk | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index a570ad7..a410ad1 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -121,6 +121,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
> @$(call MESSAGE,"Installing to staging directory")
> $($(PKG)_INSTALL_STAGING_CMDS)
> $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
> + @$(call MESSAGE,"Fixing package configuration files")
Please show the message only if there are actually files to fixup. Or
maybe just don't display any message at all. Fixing those files is kind
of a detail.
> + for file in $($(PKG)_CONFIG_FIXUP); do \
> + if [ -e $(STAGING_DIR)/usr/bin/$${file} ]; then \
No, we should not silently error out if the file doesn't exist, because
it's hiding a mistake of package .mk file. Instead, if the file doesn't
exist, we should loudly error out.
Also, please include an update to the Buildroot manual, either as part
of this patch, or as a separate patch.
Thanks for your work!
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2013-01-07 22:10 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-07 22:01 [Buildroot] new <pkg>_CONFIG_FIXUP variable Stefan Fröberg
2013-01-07 22:01 ` [Buildroot] [PATCH] new variable <pkg>_CONFIG_FIXUP Stefan Fröberg
2013-01-07 22:10 ` Thomas Petazzoni [this message]
2013-01-07 22:23 ` Stefan Fröberg
2013-01-07 22:42 ` Thomas Petazzoni
2013-01-08 20:53 ` Arnout Vandecappelle
2013-01-08 21:10 ` Thomas Petazzoni
2013-01-08 21:10 ` Arnout Vandecappelle
2013-01-07 22:10 ` [Buildroot] new <pkg>_CONFIG_FIXUP variable Yann E. MORIN
2013-01-07 22:27 ` Stefan Fröberg
2013-01-08 23:21 ` Yann E. MORIN
2013-01-09 7:54 ` Thomas Petazzoni
2013-01-09 15:21 ` Stefan Fröberg
2013-01-09 20:31 ` Arnout Vandecappelle
2013-01-10 0:16 ` Stefan Fröberg
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=20130107231037.39a798cf@skate \
--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