* [PATCH] combo-layer: make Signed-off-by optional
@ 2015-03-09 12:56 Patrick Ohly
2015-03-12 18:21 ` Paul Eggleton
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Ohly @ 2015-03-09 12:56 UTC (permalink / raw)
To: openembedded-core
It depends on the diligence of the person running the combo-layer tool
whether the Signed-off-by line added to each commit actually indicates
that the person was involved in validating the change.
When the import is purely automatic, it is better to not add the line,
because the history is more useful without it (searching for the person
really only lists changes he or she was involved with) and it would
be a false statement.
This needs to be configurable, achieved with a new global "signoff"
boolean property in combo-layer.conf, in the "DEFAULT" section.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/combo-layer | 4 +++-
scripts/combo-layer.conf.example | 6 ++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/scripts/combo-layer b/scripts/combo-layer
index 19d64e6..60ead5b 100755
--- a/scripts/combo-layer
+++ b/scripts/combo-layer
@@ -75,6 +75,8 @@ class Configuration(object):
self.parser.readfp(f)
self.repos = {}
+ self.signoff = not self.parser.has_option('DEFAULT', 'signoff') or \
+ self.parser.getboolean('DEFAULT', 'signoff')
for repo in self.parser.sections():
self.repos[repo] = {}
readsection(self.parser, repo, repo)
@@ -471,7 +473,7 @@ def apply_patchlist(conf, repos):
if os.path.getsize(patchfile) == 0:
logger.info("(skipping %d/%d %s - no changes)" % (i, linecount, patchdisp))
else:
- cmd = "git am --keep-cr -s -p1 %s" % patchfile
+ cmd = "git am --keep-cr %s-p1 %s" % ('-s ' if conf.signoff else '', patchfile)
logger.info("Applying %d/%d: %s" % (i, linecount, patchdisp))
try:
runcmd(cmd)
diff --git a/scripts/combo-layer.conf.example b/scripts/combo-layer.conf.example
index 010a692..8ad8615 100644
--- a/scripts/combo-layer.conf.example
+++ b/scripts/combo-layer.conf.example
@@ -1,5 +1,11 @@
# combo-layer example configuration file
+# global options
+[DEFAULT]
+
+# Add 'Signed-off-by' to all commits that get imported automatically.
+signoff = True
+
# component name
[bitbake]
# mandatory options
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] combo-layer: make Signed-off-by optional
2015-03-09 12:56 [PATCH] combo-layer: make Signed-off-by optional Patrick Ohly
@ 2015-03-12 18:21 ` Paul Eggleton
2015-03-12 19:45 ` Patrick Ohly
0 siblings, 1 reply; 5+ messages in thread
From: Paul Eggleton @ 2015-03-12 18:21 UTC (permalink / raw)
To: Patrick Ohly; +Cc: openembedded-core
Hi Patrick,
On Monday 09 March 2015 13:56:39 Patrick Ohly wrote:
> It depends on the diligence of the person running the combo-layer tool
> whether the Signed-off-by line added to each commit actually indicates
> that the person was involved in validating the change.
>
> When the import is purely automatic, it is better to not add the line,
> because the history is more useful without it (searching for the person
> really only lists changes he or she was involved with) and it would
> be a false statement.
>
> This needs to be configurable, achieved with a new global "signoff"
> boolean property in combo-layer.conf, in the "DEFAULT" section.
>
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
> scripts/combo-layer | 4 +++-
> scripts/combo-layer.conf.example | 6 ++++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/combo-layer b/scripts/combo-layer
> index 19d64e6..60ead5b 100755
> --- a/scripts/combo-layer
> +++ b/scripts/combo-layer
> @@ -75,6 +75,8 @@ class Configuration(object):
> self.parser.readfp(f)
>
> self.repos = {}
> + self.signoff = not self.parser.has_option('DEFAULT', 'signoff') or
> \ + self.parser.getboolean('DEFAULT', 'signoff')
> for repo in self.parser.sections():
> self.repos[repo] = {}
> readsection(self.parser, repo, repo)
> @@ -471,7 +473,7 @@ def apply_patchlist(conf, repos):
> if os.path.getsize(patchfile) == 0:
> logger.info("(skipping %d/%d %s - no changes)" % (i,
> linecount, patchdisp)) else:
> - cmd = "git am --keep-cr -s -p1 %s" % patchfile
> + cmd = "git am --keep-cr %s-p1 %s" % ('-s ' if
> conf.signoff else '', patchfile) logger.info("Applying %d/%d: %s" % (i,
> linecount, patchdisp)) try:
> runcmd(cmd)
> diff --git a/scripts/combo-layer.conf.example
> b/scripts/combo-layer.conf.example index 010a692..8ad8615 100644
> --- a/scripts/combo-layer.conf.example
> +++ b/scripts/combo-layer.conf.example
> @@ -1,5 +1,11 @@
> # combo-layer example configuration file
>
> +# global options
> +[DEFAULT]
> +
> +# Add 'Signed-off-by' to all commits that get imported automatically.
> +signoff = True
> +
> # component name
> [bitbake]
> # mandatory options
So I'm OK with adding this in as an option. However to me a name like DEFAULT
implies you're establishing a general section to apply default settings for
all components where the component can override those defaults if it chooses,
which doesn't really represent what this does - so a different name might be
more appropriate (GLOBAL or _global_ perhaps?)
Cheers,
Paul
--
Paul Eggleton
Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combo-layer: make Signed-off-by optional
2015-03-12 18:21 ` Paul Eggleton
@ 2015-03-12 19:45 ` Patrick Ohly
2015-03-13 8:57 ` Paul Eggleton
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Ohly @ 2015-03-12 19:45 UTC (permalink / raw)
To: Paul Eggleton; +Cc: openembedded-core
On Thu, 2015-03-12 at 18:21 +0000, Paul Eggleton wrote:
> Hi Patrick,
>
> On Monday 09 March 2015 13:56:39 Patrick Ohly wrote:
> > +# global options
> > +[DEFAULT]
> > +
> > +# Add 'Signed-off-by' to all commits that get imported automatically.
> > +signoff = True
> > +
> > # component name
> > [bitbake]
> > # mandatory options
>
> So I'm OK with adding this in as an option. However to me a name like DEFAULT
> implies you're establishing a general section to apply default settings for
> all components where the component can override those defaults if it chooses,
> which doesn't really represent what this does - so a different name might be
> more appropriate (GLOBAL or _global_ perhaps?)
"DEFAULT" is the special string that Python's ConfigParser uses, well,
by default for that special section which does not show up in the list
of sections. I don't know how to rename that.
I'm probably abusing this concept a bit here: it seems that special
section is meant to provide default values that get returned also for
the other sections when they don't have their own value.
Here's a cleaner solution:
* Get rid of the [DEFAULT] section in the file.
* When reading it, on-the-fly prepend the string
'[this-is-not-really-a-repo'].
* When reading global properties, get it from that section.
* When listing sections to find repos, ignore it.
How about that?
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combo-layer: make Signed-off-by optional
2015-03-12 19:45 ` Patrick Ohly
@ 2015-03-13 8:57 ` Paul Eggleton
2015-03-13 13:31 ` Patrick Ohly
0 siblings, 1 reply; 5+ messages in thread
From: Paul Eggleton @ 2015-03-13 8:57 UTC (permalink / raw)
To: Patrick Ohly; +Cc: openembedded-core
On Thursday 12 March 2015 20:45:32 Patrick Ohly wrote:
> On Thu, 2015-03-12 at 18:21 +0000, Paul Eggleton wrote:
> > On Monday 09 March 2015 13:56:39 Patrick Ohly wrote:
> > > +# global options
> > > +[DEFAULT]
> > > +
> > > +# Add 'Signed-off-by' to all commits that get imported automatically.
> > > +signoff = True
> > > +
> > >
> > > # component name
> > > [bitbake]
> > > # mandatory options
> >
> > So I'm OK with adding this in as an option. However to me a name like
> > DEFAULT implies you're establishing a general section to apply default
> > settings for all components where the component can override those
> > defaults if it chooses, which doesn't really represent what this does -
> > so a different name might be more appropriate (GLOBAL or _global_
> > perhaps?)
>
> "DEFAULT" is the special string that Python's ConfigParser uses, well,
> by default for that special section which does not show up in the list
> of sections. I don't know how to rename that.
>
> I'm probably abusing this concept a bit here: it seems that special
> section is meant to provide default values that get returned also for
> the other sections when they don't have their own value.
>
> Here's a cleaner solution:
> * Get rid of the [DEFAULT] section in the file.
> * When reading it, on-the-fly prepend the string
> '[this-is-not-really-a-repo'].
> * When reading global properties, get it from that section.
> * When listing sections to find repos, ignore it.
>
> How about that?
Hmm, ok, I wasn't aware of this built-in "DEFAULT" section behaviour. A simple
solution then would just be to make this setting per-component and keep the
DEFAULT usage in the example config - although I do realise it's unlikely to be
very useful on a per-component basis, at least it will work in the same way as
other values we read from there.
Cheers,
Paul
--
Paul Eggleton
Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] combo-layer: make Signed-off-by optional
2015-03-13 8:57 ` Paul Eggleton
@ 2015-03-13 13:31 ` Patrick Ohly
0 siblings, 0 replies; 5+ messages in thread
From: Patrick Ohly @ 2015-03-13 13:31 UTC (permalink / raw)
To: Paul Eggleton; +Cc: openembedded-core
On Fri, 2015-03-13 at 08:57 +0000, Paul Eggleton wrote:
> On Thursday 12 March 2015 20:45:32 Patrick Ohly wrote:
> > On Thu, 2015-03-12 at 18:21 +0000, Paul Eggleton wrote:
> > > On Monday 09 March 2015 13:56:39 Patrick Ohly wrote:
> > > > +# global options
> > > > +[DEFAULT]
> > > > +
> > > > +# Add 'Signed-off-by' to all commits that get imported automatically.
> > > > +signoff = True
> > > > +
> > > >
> > > > # component name
> > > > [bitbake]
> > > > # mandatory options
> > >
> > > So I'm OK with adding this in as an option. However to me a name like
> > > DEFAULT implies you're establishing a general section to apply default
> > > settings for all components where the component can override those
> > > defaults if it chooses, which doesn't really represent what this does -
> > > so a different name might be more appropriate (GLOBAL or _global_
> > > perhaps?)
> >
> > "DEFAULT" is the special string that Python's ConfigParser uses, well,
> > by default for that special section which does not show up in the list
> > of sections. I don't know how to rename that.
> >
> > I'm probably abusing this concept a bit here: it seems that special
> > section is meant to provide default values that get returned also for
> > the other sections when they don't have their own value.
> >
> > Here's a cleaner solution:
> > * Get rid of the [DEFAULT] section in the file.
> > * When reading it, on-the-fly prepend the string
> > '[this-is-not-really-a-repo'].
> > * When reading global properties, get it from that section.
> > * When listing sections to find repos, ignore it.
> >
> > How about that?
>
> Hmm, ok, I wasn't aware of this built-in "DEFAULT" section behaviour. A simple
> solution then would just be to make this setting per-component and keep the
> DEFAULT usage in the example config - although I do realise it's unlikely to be
> very useful on a per-component basis, at least it will work in the same way as
> other values we read from there.
Okay, I'll do that. It has the additional benefit of introducing the
[DEFAULT] section in the example. It may be useful also for other
properties, like a "branch = master".
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-13 13:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 12:56 [PATCH] combo-layer: make Signed-off-by optional Patrick Ohly
2015-03-12 18:21 ` Paul Eggleton
2015-03-12 19:45 ` Patrick Ohly
2015-03-13 8:57 ` Paul Eggleton
2015-03-13 13:31 ` Patrick Ohly
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.