Merge lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d into lp:ubuntu-archive-tools

[Łukasz Zemczak]

Only consider SRUs verified if they have the verification-done-SERIES tag, stop accepting verification-done as it's too confusing.

Only consider SRUs verified if they have the verification-done-SERIES tag, stop accepting verification-done as it's too confusing.

[Brian Murray (brian-murray)]

Please replace -SERIES with a tag - see my in-line comment.

review: Needs Fixing

[lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d]

1105. By Łukasz Zemczak on 2017-06-08

Use the series name in the bug message.

[Brian Murray (brian-murray)]

One last change, sorry!

review: Needs Fixing

[Adam Conrad (adconrad)]

Should we not be making this same change for verification-failed-SERIES while we're at it?

[Brian Murray (brian-murray)]

Using verification-failed-SERIES would make sense, but I think we should still use the verificationfailed cls for things that are tagged with 'verification-failed' or 'verification-failed-SERIES' to avoid any accidental acceptance of failed SRUs.

[lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d]

1106. By Łukasz Zemczak on 2017-06-08

Change the messaging as per Brian's comments.

[Łukasz Zemczak (sil2100)]

Thanks! Fixed. Note to self (and others): do not merge until we actually switch all the existing verification-done bugs to verification-done-$RELEASE - that's on my plate.

[lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d]

1107. By Łukasz Zemczak on 2017-06-09

Use verification-failed-RELEASE as well.

[Brian Murray (brian-murray)]

With Adam's change regarding v-failed-RELEASE a couple of other spots need changing.

review: Needs Fixing

[Brian Murray (brian-murray)]

Thinking about it some more I also think we should tag the bug verification-needed-RELEASE and verification-needed. In which case we should say in the comment "change the tag from verification-needed-RELEASE to verification-done-RELEASE". The generic verification-needed tag can just be ignored as it is just a method for people interested in doing SRU verification to find bugs and once all the bug tasks are closed it will no longer show up in searches.

[Łukasz Zemczak (sil2100)]

Ah, ok, so we want verification-failed-RELEASE then as well? Ok, will fix it up, with both verification-needed-RELEASE as well. Will have to modify the documentation then too.

[lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d]

1108. By Łukasz Zemczak on 2017-06-14

As per Brian's comments, we should switch to verification-needed- and verification-failed- as well.

[Brian Murray (brian-murray)]

I'm a bit confused by some of the tag manipulation in the latest iteration of sru-review - see my comment in-line.

[lp:~sil2100/ubuntu-archive-tools/sru-stop-using-v-d]

1109. By Łukasz Zemczak on 2017-06-19

Actually this will work better for the evil dance.

1110. By Łukasz Zemczak on 2017-06-19

Final cleanup.

[Łukasz Zemczak (sil2100)]

Brian, sadly the tag manipulation needs to be slightly confusing, please see the bug attached. We can't use the tags variable since it's something we temporary create during looping in the list for removing tags.

I did clean it up a bit more though, using btags there instead. Should be less troublesome now.

[Łukasz Zemczak (sil2100)]

(we need this ugly bug.tags = btags to happen at each branch to make sure it's done in case one of the conditionals is executed without the others)

[Brian Murray (brian-murray)]

I still think we are losing one change to the bug tags and I've added an inline comment about that.

[Łukasz Zemczak (sil2100)]

Followed up with an inline comment. No changes are lost, all is good, explanation can be found inline.

[Brian Murray (brian-murray)]

I tested this with lp-shell and agree that the two tags groups both end up having the 'v-[fd]*' tag removed.

review: Approve

=== modified file 'sru-release'
--- sru-release	2017-05-16 21:05:50 +0000
+++ sru-release	2017-06-19 07:51:29 +0000
@@ -199,8 +199,9 @@
                 print('Copied to %s-updates' % release)
                 if not options.no_bugs:
                     sru_bugs = match_srubugs(options, versions['changesfile'])
+                    tag = 'verification-needed-%s' % release
                     for sru_bug in sru_bugs:
-                        if 'verification-needed' not in sru_bug.tags:
+                        if tag not in sru_bug.tags:
                             update_sru_bug(sru_bug, pkg)
 
         # -proposed -> -security

=== modified file 'sru-report'
--- sru-report	2017-04-21 04:54:56 +0000
+++ sru-report	2017-06-19 07:51:29 +0000
@@ -334,10 +334,10 @@
                 if ('kernel-tracking-bug' in t or
                         'kernel-release-tracking-bug' in t):
                     cls += 'kerneltracking '
-                if 'verification-failed' in t:
+                if ('verification-failed' in t or
+                        'verification-failed-%s' % release in t):
                     cls += ' verificationfailed'
-                elif ('verification-done' in t or
-                      'verification-done-%s' % release in t):
+                elif 'verification-done-%s' % release in t:
                     cls += ' verified'
                     removable = False
                 elif b in broken_bugs:

=== modified file 'sru-review'
--- sru-review	2017-04-27 20:32:35 +0000
+++ sru-review	2017-06-19 07:51:29 +0000
@@ -299,23 +299,27 @@
               'Is there anything left to sponsor?' % num)
 
     if not sourcepkg or 'linux' not in sourcepkg:
+        # this dance is needed due to
+        # https://bugs.launchpad.net/launchpadlib/+bug/254901
         btags = bug.tags
-        for t in ('verification-failed', 'verification-done'):
+        for t in ('verification-failed', 'verification-failed-%s' % release,
+                  'verification-done', 'verification-done-%s' % release):
             if t in btags:
-                # this dance is needed due to
-                # https://bugs.launchpad.net/launchpadlib/+bug/254901
                 tags = btags
                 tags.remove(t)
                 bug.tags = tags
+
+        if 'verification-needed' not in btags:
+            btags.append('verification-needed')
+            bug.tags = btags
+
+        needed_tag = 'verification-needed-%s' % release
+        if needed_tag not in btags:
+            btags.append(needed_tag)
+            bug.tags = btags
+
         bug.lp_save()
 
-        if 'verification-needed' not in btags:
-            # this dance is needed due to
-            # https://bugs.launchpad.net/launchpadlib/+bug/254901
-            tags = btags
-            tags.append('verification-needed')
-            bug.tags = tags
-            bug.lp_save()
 
     text = ('Hello %s, or anyone else affected,\n\n' %
             re.split(r'[,\s]', bug.owner.display_name)[0])
@@ -346,14 +350,15 @@
     text += ('Your feedback will aid us getting this update out to other '
              'Ubuntu users.\n\nIf this package fixes the bug for you, '
              'please add a comment to this bug, mentioning the version of the '
-             'package you tested, and change the tag from verification-needed '
-             'to verification-done. If it does not fix the bug for you, '
-             'please add a comment stating that, and change the tag to '
-             'verification-failed.  In either case, details of your testing '
-             'will help us make a better decision.\n\nFurther information '
-             'regarding the verification process can be found at '
+             'package you tested and change the tag from '
+             'verification-needed-%s to verification-done-%s.'
+             'If it does not fix the bug for you, please add a comment '
+             'stating that, and change the tag to verification-failed-%s. In '
+             'either case, details of your testing will help us make a better '
+             'decision.\n\nFurther information regarding the verification '
+             'process can be found at '
              'https://wiki.ubuntu.com/QATeam/PerformingSRUVerification .  '
-             'Thank you in advance!')
+             'Thank you in advance!' % (release, release, release))
     bug.newMessage(content=text, subject='Please test proposed package')