| Submitter | Andy Whitcroft |
|---|---|
| Date | Feb. 10, 2010, 10:13 a.m. |
| Message ID | <1265796839-15820-2-git-send-email-apw@canonical.com> |
| Download | mbox | patch |
| Permalink | /patch/45005/ |
| State | Changes Requested |
| Delegated to: | Andy Whitcroft |
| Headers | show |
Comments
Andy Whitcroft wrote: > We have two copies of the handling for Bug: et al in get-ubuntu-log > only differing in whether Ignore: handling is active. Refactor this > code to share a single copy of this processing. > > Signed-off-by: Andy Whitcroft <apw@canonical.com> > --- > debian/scripts/misc/git-ubuntu-log | 45 +++++++++++++---------------------- > 1 files changed, 17 insertions(+), 28 deletions(-) > > diff --git a/debian/scripts/misc/git-ubuntu-log b/debian/scripts/misc/git-ubuntu-log > index 666e831..b7ca943 100755 > --- a/debian/scripts/misc/git-ubuntu-log > +++ b/debian/scripts/misc/git-ubuntu-log > @@ -168,6 +168,7 @@ sub changelog_input { > elsif ($pstate == 4) { > next unless /^\s*?(.*)/; > my $ignore = 0; > + my $do_ignore = 0; > my $bug = undef; > my %bugz = (); > my $k; > @@ -180,42 +181,30 @@ sub changelog_input { > $desc = $1; > > if ($desc =~ /^ *(Revert "|)UBUNTU:/) { > - while (<STDIN>) { > - $ignore = 1 if /^ *Ignore: yes/i; > - if (/^ *Bug: *(#|)(.*)/i) { > - foreach $k (split('(,|)*\s*#', $2)) { > - $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); > - } > - } > - elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { > - $bugz{$1} = 1; > - } > - elsif (/^ *(CVE-.*)/) { > - $cve = $1 > - } > - last if /^commit /; > - } > + $do_ignore = 1; > } else { > + $do_ignore = 0; > $author = $kernel_auth; > $ignore = 1 if $desc =~ /Merge /; I believe this will be not doing what we intended (hopefully not again my lack of pearl). If do_ignore is 0 then the record will not get ignored, even if it is a merge (which was intended) > - while (<STDIN>) { > - if (/^ *Bug: *(#|)(.*)/i) { > - foreach $k (split('(,|)*\s*#', $2)) { > - $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); > - } > - } > - elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { > - $bugz{$1} = 1; > - } > - elsif (/^ *(CVE-.*)/) { > - $cve = $1 > + } > + while (<STDIN>) { > + $ignore = 1 if /^ *Ignore: yes/i; If it would be possible to have it set ignore to 1 if the match is true and do_ignore is true, then it might be what we want and you can make the below more smb safe. > + if (/^ *Bug: *(#|)(.*)/i) { > + foreach $k (split('(,|)*\s*#', $2)) { > + $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); > } > - last if /^commit /; > } > + elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { > + $bugz{$1} = 1; > + } > + elsif (/^ *(CVE-.*)/) { > + $cve = $1 > + } > + last if /^commit /; > } > > $bug = join(", #", keys(%bugz)); > - if (!$ignore) { > + if (!$do_ignore || !$ignore) { Could be if (!ignore) again if the above change is possible and what is intended. -Stefan > &shortlog_entry($author, $desc, $bug, > $cve, $commit, 0); > }
On Wed, Feb 10, 2010 at 12:19:57PM +0100, Stefan Bader wrote: > > + $do_ignore = 0; > > $author = $kernel_auth; > > $ignore = 1 if $desc =~ /Merge /; > > I believe this will be not doing what we intended (hopefully not again my lack > of pearl). If do_ignore is 0 then the record will not get ignored, even if it is > a merge (which was intended) Well spotted. I'll sort that out and submit it. -apw
Patch
diff --git a/debian/scripts/misc/git-ubuntu-log b/debian/scripts/misc/git-ubuntu-log index 666e831..b7ca943 100755 --- a/debian/scripts/misc/git-ubuntu-log +++ b/debian/scripts/misc/git-ubuntu-log @@ -168,6 +168,7 @@ sub changelog_input { elsif ($pstate == 4) { next unless /^\s*?(.*)/; my $ignore = 0; + my $do_ignore = 0; my $bug = undef; my %bugz = (); my $k; @@ -180,42 +181,30 @@ sub changelog_input { $desc = $1; if ($desc =~ /^ *(Revert "|)UBUNTU:/) { - while (<STDIN>) { - $ignore = 1 if /^ *Ignore: yes/i; - if (/^ *Bug: *(#|)(.*)/i) { - foreach $k (split('(,|)*\s*#', $2)) { - $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); - } - } - elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { - $bugz{$1} = 1; - } - elsif (/^ *(CVE-.*)/) { - $cve = $1 - } - last if /^commit /; - } + $do_ignore = 1; } else { + $do_ignore = 0; $author = $kernel_auth; $ignore = 1 if $desc =~ /Merge /; - while (<STDIN>) { - if (/^ *Bug: *(#|)(.*)/i) { - foreach $k (split('(,|)*\s*#', $2)) { - $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); - } - } - elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { - $bugz{$1} = 1; - } - elsif (/^ *(CVE-.*)/) { - $cve = $1 + } + while (<STDIN>) { + $ignore = 1 if /^ *Ignore: yes/i; + if (/^ *Bug: *(#|)(.*)/i) { + foreach $k (split('(,|)*\s*#', $2)) { + $bugz{$k} = 1 if (($k ne '') and ($k =~ /[0-9]+/)); } - last if /^commit /; } + elsif (/^ *BugLink: *http.*:\/\/.*\/([0-9]+)/i) { + $bugz{$1} = 1; + } + elsif (/^ *(CVE-.*)/) { + $cve = $1 + } + last if /^commit /; } $bug = join(", #", keys(%bugz)); - if (!$ignore) { + if (!$do_ignore || !$ignore) { &shortlog_entry($author, $desc, $bug, $cve, $commit, 0); }
We have two copies of the handling for Bug: et al in get-ubuntu-log only differing in whether Ignore: handling is active. Refactor this code to share a single copy of this processing. Signed-off-by: Andy Whitcroft <apw@canonical.com> --- debian/scripts/misc/git-ubuntu-log | 45 +++++++++++++---------------------- 1 files changed, 17 insertions(+), 28 deletions(-)