Message ID | 1265796839-15820-2-git-send-email-apw@canonical.com |
---|---|
State | Changes Requested |
Delegated to: | Andy Whitcroft |
Headers | show |
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
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(-)