Right now the status field on S.D.O has these options
Active
Patch (code needs work)
Patch (code needs review)
Patch (ready to commit)
Fixed
Postponed
Closed (duplicate)
Closed (won't fix)
Closed (works as designed)
Needs maintainer response
Needs team response
Needs reporter response
Needs advisory
Needs public issue created
Ready for release
No maintainer response (unsupported)
Closed (fixed)
Closed (can be public)
We need workflow that works for both core and non core projects. For non core the normal workflow is
New ticket (status = active)
SecTeam review ticket and set it to needs maintainer response (or closes ticket for being invalid)
A patch gets uploaded, and status is set to (patch needs review)
The patch is confirmed and the issue is set to needs advisory
Once the advisory is ready, the issue is set to ready for release
The SA is issued and the status is set to closed (fixed)
For core it is a tad different, as we have core maintainers and other people reviewing the issue. From a practical standpoint, each issue gets 2 reviews.
1) From someone who did not write the patch
2) From a core maintainer
I would propose we change the status as below
Active => Needs triage
Patch (ready to commit) change to -> Patch (RTBC). - This would be used for core issues only
Needs advisory - Remove this, put a flag on each issue and on all issue views if there is an SA. This is a binary value and the SA can be done at any time.
Fixed - Remove this. It does not get used.
Closed (works as designed) - remove this, and use Closed (won't fix)
So the final listing of status would be:
- Needs triage
- Needs work
- Needs review
- Reviewed & tested by the community
- Ready for release
- Postponed
- Needs maintainer response
- Needs team response
- Needs reporter response
- Needs public followup
- Closed (fixed)
- Closed (can be public)
- Closed (duplicate)
- Closed (won't fix)
The workflow for core issues would be
Needs triage -> needs maintainer response(it may not be the maintainer that responds) -> Patch (code needs review) ->Patch (RTBC) -> Ready for release (this would be done by a core maintainer or equivalent confirming that everything is really ready to go and that at the release window they would commit the code)
Security team members (including provisional and former security team members) could set any of the above statuses. Non-security team members could only set the following: Needs Triage, Needs work, Needs review, Needs team response, and Needs reporter response.
Comments
Comment #2
mlhess commentedComment #3
mlhess commentedComment #4
greggles+1 from me.
Comment #5
drummWe'll need to be sure to update any open issues which are in removed statuses, since they will become harder to find. (The status filter to get them goes away.)
Comment #6
cashwilliams commented+1 from me with one small change:
Patch (RTBC)
Without a * on the end, this means only sec team members can mark an issue as reviewed? It seems like a maintainer should be able to do that as well.
Comment #7
xjmCan we not just use the exact same things as the public queue when they are equivalent? For those of us who work in both, it'd reduce the confusion.
Comment #8
xjmSo (in the order I'd recommend and with the correct case):
Needs triage
Needs work
Needs review
Reviewed & tested by the community
Ready for release
Postponed
Needs maintainer response
Needs team response
Needs reporter response
Needs public issue created
Closed (fixed)
Closed (can be public)
Closed (duplicate)
Closed (won't fix)
I agree with Cash about the RTBC status; I'd say that anyone on the issue should be able to mark it RTBC. Just not "Ready for release".
Edit: Also I don't see a case for non-sec-team-members to set something back to "Needs triage" but maybe I'm missing something.
Comment #9
xjmI left out "No maintainer response (unsupported)" I guess. Is that the status used when going through the process of marking a module unsupported? Is it a closed status or does the issue get moved to a different closed status after that?
Comment #10
xjmAnother question. Is "Closed (can be public)" necessary? Can it just be "Needs public issue created" -> "Closed (fixed)"? Or is "Closed (fixed)" only used for issues attached to an SA? In core we use "Fixed" (and therefore "Closed (fixed)")
Also, colors to change:
Needs triage -- public "Active" white seems like it would still make sense.
Ready for release -- maybe public "Fixed" green, if s.d.o does not auto-close fixed issues?
Needs maintainer response -- this is currently the same dark pink as "Closed (fixed)" which always confuses me; can we also make it white?
Closed (can be public) -- If this status is needed, please make it the same dark pink as "Closed (fixed)" since it's a positive resolution.
Comment #11
xjmMore questions. Can "Needs public issue created" be renamed and broadened to "Needs public followup"? There are LOTS of public followups we need, core especially but also other things, that are not necessarily dropping the whole thing in public, but followups related to them that need to exist.
Comment #12
xjmUpdating the IS with my proposal from #8.
Comment #13
cashwilliams commentedI think we might need one (or two) more status related to actually doing a release.
"Ready for release" is a bit unclear. Is it ready for the maintainer to commit the code or ready for the sec team to publish the SA.
We need an additional set to make it clear what is waiting on the maintainer to commit (and waiting on Wednesday to come around) and then another to make it clear the issue is committed and needs the SA published.
Especially true because if something _was_ committed, I don't want to miss it and not publish the SA.
Comment #14
greggles+1 to #13.
Comment #15
xjmWouldn't "ready for the maintainer to commit" just be RTBC? Or are you saying a separate status after RTBC that only sec team members could set?
Comment #16
xjmForgot one part of my earlier proposal (Needs public followup); updating the IS.
Comment #17
xjmEdited the current list rather than the proposed one; fixing that.
Comment #18
mlhess commentedSo I think the final list is:
Needs triage
Needs work
Needs review
Needs maintainer response
Needs team response
Needs reporter response
Needs public followup
Reviewed & tested by the community
Ready for SA to be Published
No maintainer response (unsupported)
Postponed
Closed (fixed)
Closed (can be public)
Closed (duplicate)
Closed (won't fix)
I think we will implement this tomorrow.
@cash/@xjm, I renamed ready to release for Ready for SA to published. We can use RTBC for ready to commit.
Lets get these changes done made and then we can add the complexity of who can do what change.