Hierarchical release note feature functions #435

roycaihw · 2017-10-09T00:57:12Z

This is the second PR following #434. Please refer to #434 to review the design doc and test commands.

roycaihw · 2017-10-09T00:57:38Z

yutongz · 2017-10-10T01:55:53Z

toolbox/relnotes/hierarchy.go

+	"github.com/google/go-github/github"
+)
+
+func hierarchicalNoteLayout(f *os.File, dict map[string]map[string]map[int]map[int]bool, issueMap map[int]*github.Issue) {


This dict is crazy, you may want to split them, and create a structure for each level.

@yutongz Updated.

jennybuckley · 2017-10-10T22:01:25Z

toolbox/relnotes/hierarchy.go

+	for _, pr := range prs {
+		issues := extractFixedIssues(*issueMap[pr].Body)
+		if len(issues) == 0 {
+			setNoteDict(dict, "nullSig", "nullArea", -1, pr)


I think we could use extractIssueSIGs(issueMap[pr]) and extractIssueArea(issueMap[pr]) instead of "nullSig" and "nullArea" to still sort these issues, since many prs don't link to an issue.

@jennybuckley In our design doc we want the automation to enforce every release-note PR linking to at least one issue. But I agree that the rule seems to be not applied yet. I pushed an update to use SIG & area labels from PRs with no issue associated.

jennybuckley · 2017-10-10T22:17:10Z

toolbox/relnotes/hierarchy.go

+	var areas = make([]string, 0)
+	for _, l := range i.Labels {
+		if strings.HasPrefix(*l.Name, "area/") {
+			areas = append(areas, (*l.Name)[5:])


Does github api guarantee anything about the order of the labels returned? I am wondering because if two issues have labels "area/Alpha" and "area/Beta" and the labels can be in any order, then they might be put into different area buckets in the final document, even though they have the same areas

@jennybuckley I think it guarantees alphabetical order.

Also TrimPrefix here.

xiangpengzhao · 2017-10-20T16:41:33Z

toolbox/relnotes/hierarchy.go

+	var areas = make([]string, 0)
+	for _, l := range i.Labels {
+		if strings.HasPrefix(*l.Name, "area/") {
+			areas = append(areas, (*l.Name)[5:])


Also TrimPrefix here.

xiangpengzhao · 2017-10-20T16:44:02Z

toolbox/relnotes/hierarchy.go

+	var sigs = make([]string, 0)
+	for _, l := range i.Labels {
+		if strings.HasPrefix(*l.Name, "sig/") {
+			sigs = append(sigs, (*l.Name)[4:])


Why not using TrimPrefix here?

xiangpengzhao · 2017-10-20T17:02:59Z

toolbox/relnotes/hierarchy.go

+			// with at least one issue. However it's observed that the rule is not
+			// applied yet. Also old PRs may not link to an issue.
+			//
+			// To produce info-richer release note, we try to get SIG and Area label


Unfortunately, most of PRs don't have either sig or area label. I once felt a bit painful when I wanted to filter PRs by sig. I was thinking we enforce sig labels on PRs as we already do on issues. Then we may get more info here. @enisoc @Bradamant3 WDYT?

Enforcing sig label on PR will also be helpful for #348.

xiangpengzhao · 2017-10-20T17:09:07Z

toolbox/relnotes/hierarchy.go

+// extractFixedIssues parses the fixed issues' id from PR body.
+func extractFixedIssues(msg string) []int {
+	var issues = make([]int, 0)
+	re, _ := regexp.Compile("fixes #([0-9]+)")


note that github not only supports fixes but also fixed, closes, closed, etc, though we use fixes in PR template. That means, some guys may use other supported keywords.

and it can also fixes issue in other repo, e.g., fixes kubernetes/kubeadm#123. do we need to respect this case?

enisoc · 2017-11-21T18:33:21Z

@roycaihw #434 has been merged. Is this in working order if I patch it in locally? I'd like to be able to do a manual run as soon as possible, to start showing the relnotes lead (@Bradamant3) what the output looks like.

xiangpengzhao · 2017-11-21T18:43:17Z

@enisoc I don't think so. I don't find hierarchicalNoteLayout or createHierarchicalNote in this PR being invoked anywhere. I guess they need to be invoked somewhere in the code in #434.
@roycaihw for confirmation.

roycaihw · 2017-11-21T18:44:48Z

@enisoc @xiangpengzhao There are some changes needed. I will do an update soon today.

…-release

Check issue != pr in extracting fixed issues; Look for "ref" besides "fixes"

roycaihw · 2017-11-21T22:03:00Z

@enisoc @xiangpengzhao I've updated this PR. Now it rebases on #434 and generates layered output. One common problem I hit during testing was PRs referring to issues in other repo (e.g kubernetes/kubeadm). Currently the tool doesn't recognize those issues.

Example output against v1.7.0..v1.7.2
https://gist.github.com/roycaihw/01950b80767620902edd23e874d6b749

xiangpengzhao · 2017-11-23T03:37:15Z

@roycaihw the hierarchy of the example output looks great, but it's a bit strange to see Null* things.

xiangpengzhao · 2017-11-28T18:57:04Z

@roycaihw I run ../release/bazel-bin/toolbox/relnotes/relnotes --html-file /tmp/release-note-html-testfile --full against release-1.9 branch but I just get below result. Anything am I missing?

cc @enisoc @Bradamant3 @nickchase

# release-1.9

[Documentation](https://docs.k8s.io) & [Examples](https://releases.k8s.io/release-1.9/examples)

## Major Themes

* TBD

## Other notable improvements

* TBD

## Known Issues

* TBD

## Provider-specific Notes

* TBD

### Previous Release Included in 1d261b5d48155df9e1db90929b8615ba11696c3d

xiangpengzhao · 2017-11-28T18:58:05Z

logs:

cloud@ubuntu:~/go/src/k8s.io/kubernetes$ ../release/bazel-bin/toolbox/relnotes/relnotes --html-file /tmp/release-note-html-testfile --full
2017/11/29 02:53:34 Boolean flags: full: true, htmlize-md: false, preview: false, quiet: false
2017/11/29 02:53:34 Input branch range: 
2017/11/29 02:53:34 Working branch: release-1.9. Branch version suffix: -1.9.
2017/11/29 02:53:34 Output markdown file path: /tmp/release-notes-release-1.9.md
2017/11/29 02:53:34 Output HTML file path: /tmp/release-note-html-testfile
2017/11/29 02:53:34 Gathering release commits from Github...
2017/11/29 02:53:47 Gathering "release-note" labelled PRs using Github search API. This may take a while...
2017/11/29 02:55:03 "release-note" labelled PRs gathered.
2017/11/29 02:55:03 Gathering "release-note-action-required" labelled PRs using Github search API.
2017/11/29 02:55:07 "release-note-action-required" labelled PRs gathered.
2017/11/29 02:55:07 Generating release notes...
2017/11/29 02:55:07 Checking if draft release notes exist for 1d261b5d48155df9e1db90929b8615ba11696c3d...
2017/11/29 02:55:08 Failed to find draft - creating generic template... (error message/status code printed below)
2017/11/29 02:55:08 Response status code: 404
2017/11/29 02:55:09 Preparing layout...
2017/11/29 02:55:09 Generating HTML release note...
2017/11/29 02:55:10 Displaying the markdown release note to stdout...

xiangpengzhao · 2017-11-28T19:06:08Z

BTW, it works well for release-1.8.

roycaihw · 2017-11-28T19:29:44Z

@xiangpengzhao The --full flag is used to force a minor release instead of a patch release. In a minor release, the tool looks for the release note draft (https://github.com/kubernetes/features/blob/master/release-1.9/release-notes-draft.md), which doesn't exist for branch 1.9. So the tool creates a generic template of release note draft.

If you run the command without --full, it will show the PRs between last release to HEAD on branch release-1.9

enisoc · 2017-11-29T01:37:19Z

@roycaihw At the moment, we're trying to use the generator to make a starting point for the hand-written 1.9.0 minor release notes. It's a little different than the way anago uses it.

@nickchase @xiangpengzhao @Bradamant3 I think I found a process to generate what we need.

Here's the output I get:

https://gist.github.com/enisoc/6389e785c7ff8b73f473a5ca5c222a78

For our purpose, maybe we can clean this up by using a couple layers of markdown headers to reduce the nesting of bulleted lists. @roycaihw Do you think that would be an easy change to make?

My steps:

# prereqs:
#   https://github.com/golang/dep
#   https://bazel.build/
#   https://help.github.com/articles/creating-a-personal-access-token-for-the-command-line/

# in kubernetes/release repo
git fetch upstream pull/435/head:pull-435
git checkout pull-435
dep ensure
bazel run //:gazelle
bazel build toolbox/relnotes:relnotes

# in kubernetes/kubernetes repo
export GITHUB_TOKEN=...
../release/bazel-bin/toolbox/relnotes/relnotes \
  --htmlize-md --quiet \
  --markdown-file=/tmp/relnotes.md \
  --html-file=/tmp/relnotes.html \
  --branch=master \
  v1.8.0..

xiangpengzhao · 2017-11-29T01:53:14Z

@enisoc cool ! yeah what you get are the full release notes of 1.9 since 1.8.0. That's what we want. 👍

I try my original command w/o --full (i.e., ../release/bazel-bin/toolbox/relnotes/relnotes --html-file /tmp/release-note-html-testfile) as @roycaihw mentioned, it only gets the release notes since v1.9.0-alpha.2

xiangpengzhao · 2017-11-29T01:57:57Z

For our purpose, maybe we can clean this up by using a couple layers of markdown headers to reduce the nesting of bulleted lists.

agreed. another concern is that the output now including both PRs and their associated issues. It's not so clear to identify what are PRs and what are issues in the output.

lists.

roycaihw · 2017-11-29T21:46:26Z

For our purpose, maybe we can clean this up by using a couple layers of markdown headers to reduce the nesting of bulleted lists.

@enisoc Yes. I just made a simple change. Here is a sample output: https://gist.github.com/roycaihw/62279aa12a6d7cbcb9a9d24dd4b0bbad

@xiangpengzhao Sorry I wasn't aware of your goal. Yes you need to specify the range you want to get the changelog. When you omit the range, the start point would be the last release on that branch (in your case v1.9.0-alpha.2) and the end point would be current HEAD.

fejta-bot · 2018-02-27T22:27:50Z

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 · 2018-03-29T23:14:52Z

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
/remove-lifecycle stale

fejta-bot · 2018-04-28T23:31:05Z

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

release lead role handbook: encourage global mindset

Implement hierarchical release note feature.

d020597

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 9, 2017

yutongz reviewed Oct 10, 2017

View reviewed changes

Use type alias instead of nested map.

3f67cdb

jennybuckley reviewed Oct 10, 2017

View reviewed changes

Generalize for PRs with no issue and PRs not following template.

3b596e6

enisoc mentioned this pull request Oct 17, 2017

Generate hierarchical release notes kubernetes/sig-release#20

Closed

xiangpengzhao reviewed Oct 20, 2017

View reviewed changes

xiangpengzhao mentioned this pull request Nov 14, 2017

issue checker is confused by PRs referencing other PRs kubernetes/test-infra#5483

Closed

roycaihw mentioned this pull request Nov 20, 2017

Address markdown format symbols in PR release note block? kubernetes/sig-release#29

Closed

roycaihw added 2 commits November 21, 2017 10:50

Merge branch 'master' of github.com:kubernetes/release into hierarchy…

a05dc4c

…-release

Apply hierarchical release note to main function, and minor fixes:

4c4eb7d

Check issue != pr in extracting fixed issues; Look for "ref" besides "fixes"

Format change: use markdown headers to reduce the nesting of bulleted

313b952

lists.

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 27, 2018

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 29, 2018

k8s-ci-robot closed this Apr 28, 2018

marpaia pushed a commit to marpaia/release that referenced this pull request Feb 21, 2019

Merge pull request kubernetes#435 from tpepper/master

c1972b5

release lead role handbook: encourage global mindset

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hierarchical release note feature functions #435

Hierarchical release note feature functions #435

roycaihw commented Oct 9, 2017 •

edited

Loading

roycaihw commented Oct 9, 2017

yutongz Oct 10, 2017

roycaihw Oct 10, 2017

jennybuckley Oct 10, 2017

roycaihw Oct 11, 2017

jennybuckley Oct 10, 2017

roycaihw Oct 11, 2017 •

edited

Loading

xiangpengzhao Oct 20, 2017 •

edited

Loading

xiangpengzhao Oct 20, 2017 •

edited

Loading

xiangpengzhao Oct 20, 2017

xiangpengzhao Oct 20, 2017

xiangpengzhao Oct 21, 2017

xiangpengzhao Oct 20, 2017

xiangpengzhao Oct 20, 2017

enisoc commented Nov 21, 2017

xiangpengzhao commented Nov 21, 2017

roycaihw commented Nov 21, 2017

roycaihw commented Nov 21, 2017

xiangpengzhao commented Nov 23, 2017

xiangpengzhao commented Nov 28, 2017 •

edited

Loading

xiangpengzhao commented Nov 28, 2017

xiangpengzhao commented Nov 28, 2017

roycaihw commented Nov 28, 2017

enisoc commented Nov 29, 2017

xiangpengzhao commented Nov 29, 2017

xiangpengzhao commented Nov 29, 2017

roycaihw commented Nov 29, 2017

fejta-bot commented Feb 27, 2018

fejta-bot commented Mar 29, 2018

fejta-bot commented Apr 28, 2018

Hierarchical release note feature functions #435

Hierarchical release note feature functions #435

Conversation

roycaihw commented Oct 9, 2017 • edited Loading

roycaihw commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roycaihw Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

xiangpengzhao Oct 20, 2017 • edited Loading

Choose a reason for hiding this comment

xiangpengzhao Oct 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enisoc commented Nov 21, 2017

xiangpengzhao commented Nov 21, 2017

roycaihw commented Nov 21, 2017

roycaihw commented Nov 21, 2017

xiangpengzhao commented Nov 23, 2017

xiangpengzhao commented Nov 28, 2017 • edited Loading

xiangpengzhao commented Nov 28, 2017

xiangpengzhao commented Nov 28, 2017

roycaihw commented Nov 28, 2017

enisoc commented Nov 29, 2017

xiangpengzhao commented Nov 29, 2017

xiangpengzhao commented Nov 29, 2017

roycaihw commented Nov 29, 2017

fejta-bot commented Feb 27, 2018

fejta-bot commented Mar 29, 2018

fejta-bot commented Apr 28, 2018

roycaihw commented Oct 9, 2017 •

edited

Loading

roycaihw Oct 11, 2017 •

edited

Loading

xiangpengzhao Oct 20, 2017 •

edited

Loading

xiangpengzhao Oct 20, 2017 •

edited

Loading

xiangpengzhao commented Nov 28, 2017 •

edited

Loading