search

kubernetes / test-infra

Test infrastructure for the Kubernetes project.
Apache License 2.0
3.83k stars 2.65k forks source link

prow: dev-release-note functionality #5843

Closed munnerz closed 5 years ago

munnerz commented 6 years ago

sig-api-machinery are trying to improve the developer experience when building Kubernetes. We currently have a list of >100 PRs that do not specify release notes as they are solely developer facing changes, however this has caused many problems resulting in end users needing to look through recently merged PRs in order to work out how to fix their own code.

Ideally, there'd be some way for users to specify a 'developer' release note when writing PRs. After some discussion, @sttts suggested something along the lines of a new dev-release-note tag that is optional for all PRs (in order to not require all users to specify a dev-release-note on existing PRs).

Ideally we'd then have some kind of regex that matches directories, and enforces a dev-release-note for PRs that touch files matching the regex. It'd be possible to specify NONE just like with release-note still (for cases where it does not make sense to include a dev-release-note).

Is this something that can be considered to be included in Prow, and have sig-testing got any thoughts on how this might/should look?

/cc @sttts @nikhita @BenTheElder @fejta @spiffxp

BenTheElder commented 6 years ago

If we're allowing NONE what purpose does the regex serve? Why not just put it in the PR template for all PRs?

On Wed, Dec 6, 2017, 09:14 James Munnelly notifications@github.com wrote:

sig-api-machinery are trying to improve the developer experience when building Kubernetes. We currently have a list of >100 PRs that do not specify release notes as they are solely developer facing changes, however this has caused many problems resulting in end users needing to look through recently merged PRs in order to work out how to fix their own code.

Ideally, there'd be some way for users to specify a 'developer' release note when writing PRs. After some discussion, @sttts https://github.com/sttts suggested something along the lines of a new dev-release-note tag that is optional for all PRs (in order to not require all users to specify a dev-release-note on existing PRs).

Ideally we'd then have some kind of regex that matches directories, and enforces a dev-release-note for PRs that touch files matching the regex. It'd be possible to specify NONE just like with release-note still (for cases where it does not make sense to include a dev-release-note).

Is this something that can be considered to be included in Prow, and have sig-testing got any thoughts on how this might/should look?

/cc @sttts https://github.com/sttts @nikhita https://github.com/nikhita @BenTheElder https://github.com/bentheelder @fejta https://github.com/fejta @spiffxp https://github.com/spiffxp

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kubernetes/test-infra/issues/5843, or mute the thread https://github.com/notifications/unsubscribe-auth/AA4Bq-JSL-hQiPkxa932n0gY8MkAn1iDks5s9stdgaJpZM4Q4RxX .

munnerz commented 6 years ago

Sure - I didn't want to be too intrusive to the existing process is all (we end out with something that looks like this comment everywhere)

Release note:

NONE

Developer release note:

NONE
BenTheElder commented 6 years ago

/cc @cblecker @grodrigues3 @kubernetes/sig-contributor-experience-feature-requests /kind feature

cblecker commented 6 years ago

A couple questions:

sttts commented 6 years ago

"Developers building atop the API (e.g., client libraries, CRDs)"

This.

If so, I also assume that these "Dev Release Notes" would need to be published? How often (same release cycle)?

In every release of Kubernetes which has a matching release of our published libraries, e.g. client-go, apimachinery, etc. We would put the release into the Github release/tag markdown.

BenTheElder commented 6 years ago

also FYI @cjwagner since this would touch on Tide / Prow

I think this is certainly possible, but before implementing we should have a specific proposal and loop in ContribEx, when you say enforcing do you mean not allowing merge without a value? If so we should probably just add it to all PRs and handle it the same as the existing release note process. Edit: handle it from the infra side, obviously the labels etc wouldn't be exactly the same..

cblecker commented 6 years ago

Based on @sttts's answers, I'd suggest this would be more a topic for @kubernetes/sig-release-feature-requests then for contribex.

I would also suggest that perhaps there is a way to flag a release note in the text as being applicable to devs (similar to action required), and use the same release note system we already have.

sttts commented 6 years ago

/cc @caesarxuchao

caesarxuchao commented 6 years ago

cc @roycaihw who wrote a doc for similar ideas: https://docs.google.com/document/d/1AlcDeTlX7JOLkC7DEVtT9LRZ8N1bK1hrp8LRuep5joQ/edit#heading=h.5irk4csrpu0y

sttts commented 6 years ago

@caesarxuchao @roycaihw awesome document. Fits perfectly here.

BenTheElder commented 6 years ago

@caesarxuchao @sttts that looks pretty good to me, I think the problem is that this requires prow jobs to have GitHub API access.

Possibly we could do this the way @fejta-bot works, which is to just comment on the PR with /command foo with a token / account that has no special privileges and then have prow convert this to adding a label. The downside to this is that we wouldn't be restricting the ability to add these labels.

5848 might be another option. /cc @cjwagner

sttts commented 6 years ago

Possibly we could do this the way @fejta-bot works

Where is the current release-note logic? Can't we re-use that code?

BenTheElder commented 6 years ago

Where is the current release-note logic? Can't we re-use that code?

It's in prow/plugins/releasenote, but it's not a prow job and I don't think it inspects the file contents so the above doc wouldn't quite match up. It would certainly be more straight forward to do a prow plugin / extend the releasenote plugin but the sticking point would probably be needing to grep godeps.

I'm pretty sure the existing logic doesn't even inspect the file names in the PR @cjwagner .

sttts commented 6 years ago

I don't think it inspects the file contents

got it.

fejta-bot commented 6 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

nikhita commented 6 years ago

/remove-lifecycle stale

I'd suggest this would be more a topic for @kubernetes/sig-release-feature-requests then for contribex.

If we were to get this in, what should be the next step? Talk to sig-release and sig-contribex?

BenTheElder commented 6 years ago

An aside: I think now something like this would be a good target for an external plugin.

nikhita commented 6 years ago

Reviving this again:

If we were to get this in, what should be the next step? Talk to sig-release and sig-contribex?

nikhita commented 6 years ago

I'm happy to work on the automation bits for this if proper guidelines are first agreed upon. :)

Right now, we go through all PRs created against client-go, apimachinery, etc in staging in k/k, check if the code change is relevant enough for a release note, write the release note in a spreadsheet and then convert it to markdown.

I just created the spreadsheet for release-8 for client-go. It has 201 PRs! :scream:

BenTheElder commented 6 years ago

Hi yes, talking to contribex and release about how this should work would be good. I should have commented the first time instead of +1ing 🙃

nikhita commented 6 years ago

Hi yes, talking to contribex and release about how this should work would be good. I should have commented the first time instead of +1ing upside_down_face

:+1: :slightly_smiling_face:

fejta-bot commented 6 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

nikhita commented 6 years ago

/remove-lifecycle stale

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 5 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

sttts commented 5 years ago

/remove-lifecycle rotten

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 5 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

fejta-bot commented 5 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /close

k8s-ci-robot commented 5 years ago

@fejta-bot: Closing this issue.

In response to [this](https://github.com/kubernetes/test-infra/issues/5843#issuecomment-506468814): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >Send feedback to sig-testing, kubernetes/test-infra and/or [fejta](https://github.com/fejta). >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.