Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

github: switch issue templates to new labels #10266

Closed

Conversation

mspncp
Copy link
Contributor

@mspncp mspncp commented Oct 25, 2019

(To be sure about the syntax, I created a test template using the wizard.)

@mspncp mspncp requested a review from levitte October 25, 2019 22:19
@mspncp mspncp added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 26, 2019
openssl-machine pushed a commit that referenced this pull request Oct 27, 2019
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #10266)
@mspncp
Copy link
Contributor Author

mspncp commented Oct 27, 2019

Merged to cfa71c5 and removed the [feature] and the [bug] label.

@mspncp mspncp closed this Oct 27, 2019
@mspncp mspncp deleted the pr-change-issue-template-labels branch October 27, 2019 17:31
@mattcaswell
Copy link
Member

I notice that you removed the "approval: done" label when you added "ready to merge"...is that they way we should do things - or should we keep the "approval: done" after "ready to merge" is additionally added? To me it makes more sense to keep it.

@mspncp
Copy link
Contributor Author

mspncp commented Oct 28, 2019

I wasn’t sure myself whether I should remove it or not. If you prefer to keep it, I‘m ok with it.

@mattcaswell
Copy link
Member

To me the "approval: done" status is not something that is lost simply because we have additionally gained the "ready to merge" status.

@mspncp
Copy link
Contributor Author

mspncp commented Oct 28, 2019

Ok, then I'll restore the label on the recent tickets.

(FWIW: bulk adding/removal of labels is possible using multiselection on the issue search page.)

@mspncp mspncp added the approval: done This pull request has the required number of approvals label Oct 28, 2019
@mspncp
Copy link
Contributor Author

mspncp commented Oct 28, 2019

To me the "approval: done" status is not something that is lost simply because we have additionally gained the "ready to merge" status.

P.S: The motivation behind removing the 'approval: done' label was to consider 'approval: *' and 'ready for merge' as mutual exclusive ticket states. But viewing them as independent properties makes equally sense.

Anyway, I think the @openssl/omc needs to state a clear policy for handling the 'approval: *' and 'ready to merge' labels, in particular in the case when the 'approval: ready' state needs to be withdrawn (e.g. because there was a force push after 'ready to merge'). Partial support by a GitHub bot might be helpful, but not sufficient for all use cases.

@mspncp mspncp removed the approval: done This pull request has the required number of approvals label Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants