Code review comment for ~rodrigo-zaiden/ubuntu-cve-tracker:security-signoff-add-assignee

Hi Steve,

> Apologies for the delayed review. I think the idea is okay; I should note that
> the kernel-sru-script that reports pending signoffs assigns the bug task to
> the person running the script as well (see https://git.launchpad.net/ubuntu-
> qa-tools/tree/security-tools/kernel-sru-check#n106 ) but only if the status is
> "Confirmed".

thanks for pointing this. I completely missed this part of the kernel-sru-check script.
One thing that caught my attention when re-reading the kernel-sru-workflow is that it seems like the "Security-signoff" task only moves to status "Confirmed" when "Verification-testing" task is "Fix Released", but I've seen cases where "Verification-testing" is "Invalid" and so, "Security-signoff" is not being moved to "Confirmed". Maybe that is something we can raise with the kernel team if this is a valid statement.

step10:
    Once all the bugs, requiring verification, listed in the changelog have been marked
    verification-done-<series>, the kernel team changes the state of the Verification-testing
    task to completed (status: Fix Released). Workflow Mgr. then detects this and sets the
    Certification-testing, Regression-testing and Security-signoff tasks to the ready-to-start
    state (status: Confirmed).

>
> (Really, that script should probably be moved to UCT, there's additional
> improvements I'd like to make to it that would be a little easier if it was in
> UCT and not in ubuntu-qa-tools.)
>
> I guess what I'm more interested is how you see this being used in the signoff
> / tracking process. AFAICT, it cannot be used independently of changing the
> task state either acking or nacking it for the security pocket; it might be
> worth being possible change the assignee independently so that at the
> beginning of signoffs, you could change self-assign for kernels you're
> starting the sign-off process on, in part to indicate you're starting on them.

Well, considering that I was not aware of the current kernel-sru-check script validation on the assignee, I was simply thinking that once someone is signing kernels off (that is, marking "Fix Released" or "Invalid" despite the current status), it would be useful to have that person being assigned to the task because we had cases where you were the assignee and I was doing the sign-off and the task remained assigned to you after the status was changed.
Now, I think that, this is probably a case where the "Security-signoff" task was not "Confirmed" and one of two things happened:
1. I was ahead of time when I wanted to sign things off and didn't properly wait until the task landed in the correct state.
2. "Security-signoff" never changed to "Confirmed" because "Verification-testing" didn't change to "Fix Released".

if any of these 2 happened and the second can be adjusted in the workflow, I think I can simply drop this merge as kernel-sru-script handles the assignee.

When I wrote this change, I was not thinking of using it independently, but I agree it could be used with a different purpose like re-assigning and changing to "In Progress".

For now, I'll move this to "Work in Progress" to make adjustments to be able to change the assignee independently of the status being changed (ack/nck).
But I would like to hear from you what is your opinion on the "Security-signoff" status not being moved to "Confirmed", if we have more scripts and places needing fixes, I could invalidate this merge and propose a new one moving kernel-sru-check to UCT and other changes on this context all together.

Thanks a lot for checking it