Zephyr: GH labels reorg

Created on 3 Apr 2019  路  16Comments  路  Source: zephyrproject-rtos/zephyr

Following https://github.com/zephyrproject-rtos/zephyr/pull/15054 (Assuming it is merged),
this issue aims at settling other fixes for the labels which were not trivially agreed yet.
I list each proposal in a separate point below, if you agree just give it a thumbs-up, if you disagree a down (and create a separate comment for it). Maybe we can handle the discussion just in this fashion.

Enhancement Process

Most helpful comment

Create labels "area:Security" and "area:API"
Purpose: For these 2 "areas" to have their own labels, separate from the meetings labels

All 16 comments

Create/Rename labels for issues to be discussed in a following meeting,
meeting: {TSC|Securty|API|dev-review}

Purpose: To clearly communicate to the issue/PR author that the issue will probably be discussed there.
To have a quick filter to be used in the meeting, of issues which are meant to be discussed there.
To avoid confusion with an issue which is only related to the APIs or Security area (but not meant to be discussed in the meeting)

Rename the "TSC", "Maintainer", "Trivial" & "Security" labels into
review time: {4h (trivial)| 2d (maintainer)| 3d | 1w (contentious)}
(Also update the figure and text in https://docs.zephyrproject.org/latest/development_process/review_process.html? highlight=review%20time , to reflect this new naming)

Purpose: To decouple what needs to be discussed in a meeting, and how long something should be reviewed. Hopefully avoiding some confusion.

Create labels "area:Security" and "area:API"
Purpose: For these 2 "areas" to have their own labels, separate from the meetings labels

CC (ran out of assignments): @jhedberg , @jfischer-phytec-iot , @nategraff-sifive , @mgielda , @ruuddw , @smijovic-si

Great work! +1 to all the above. I'd like to propose for up/down voting:

Remove DNM label and just use "Draft" PRs: https://github.blog/2019-02-14-introducing-draft-pull-requests/

Remove DNM label and just use "Draft" PRs

The thing with this one, is that you cannot convert a normal PR into draft. While quite a few times, DNM is added to a PR for some reason afterwards. I would promote people using Draft PRs, but would not remove the DNM label.

I would promote people using Draft PRs, but would not remove the DNM label.

Fair enough; I didn't know that.

Remove DNM label and just use "Draft" PRs

The thing with this one, is that you cannot convert a normal PR into draft. While quite a few times, DNM is added to a PR for some reason afterwards. I would promote people using Draft PRs, but would not remove the DNM label.

Yes, I think this is a good approach.

(and create a separate comment for it). Maybe we can handle the discussion just in this fashion.

Unfortunately github issues don't seem to support any form of threading, so while voting works, discussing particular changes doesn't really seem posssible here :-( On the other hand you could stack a modification PR on top of the "current state" PR #15054
https://graysonkoonce.com/stacked-pull-requests-keeping-github-diffs-small/
Or even better: just expedite PR #15054

To clearly communicate to the issue/PR author that the issue will probably be discussed there.

Can it also be set by the author to request discussion in the meeting?

This is a common problem with a number of labels: From: whom To: whom? This needs to be clarified in the doc, hopefully on a per prefix basis to keep things consistent and simple.

Or even better: just expedite

That depends only on people approving that PR, and somebody merging it. Today both @carlescufi & @nashif have old addressed rejections which are pending their re-review..
@marc-hb If you like that PR, you can show so by approving it. If you like any of the proposals above you can indicate it with a thumb-up

Can it also be set by the author to request discussion in the meeting?

Yes, at least for collaborators it should be possible.

Rename the "TSC", "Maintainer", "Trivial" & "Security" labels into
review time: {4h (trivial)| 2d (maintainer)| 3d | 1w (contentious)}
(Also update the figure and text in https://docs.zephyrproject.org/latest/development_process/review_process.html? highlight=review%20time , to reflect this new naming)

Purpose: To decouple what needs to be discussed in a meeting, and how long something should be reviewed. Hopefully avoiding some confusion.

I'm not sure renaming TSC/Maintainer to a review time is useful. The intent of the label is to highlight who needs to pay attention. I know the TSC label gets used a good bit in the TSC meetings.

Should we close? Looks like the process PR was merged. Nice work @aescolar!

Closing this as further progress seems very unlikely.

@mbolivar That PR was a precursor to what was proposed here. None of these proposals has been taken in.

Closing this as further progress seems very unlikely.

Is this disappointment at the low number of votes, lack of consensus off-line or just lack of time? In the first case and now that PR #15054 has been merged (thx!), I hope following-up with more "traditional" PRs would get more attention than this issue and its less "familiar" voting system.

@marc-hb lack of consensus.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

akansal1 picture akansal1  路  4Comments

skylayer picture skylayer  路  4Comments

karstenkoenig picture karstenkoenig  路  4Comments

nashif picture nashif  路  5Comments

ghost picture ghost  路  4Comments