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.

Sign up for GitHub

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

Jump to bottom

Update the release GitHub page from krel #1789

Merged
merged 2 commits into from
Nov 25, 2020
Merged

Update the release GitHub page from krel #1789

merged 2 commits into from
Nov 25, 2020

Conversation

puerco
Copy link
Member

@puerco puerco commented Nov 25, 2020

What type of PR is this?

/kind feature

What this PR does / why we need it:

This pull request integrates the new announce code that updates the GitHub page to krel release.

The github_page.go code in the announce pkg has been modified to avoid it failing in mock mode, when the release tag does not exist.

Which issue(s) this PR fixes:

Part of #1673

Special notes for your reviewer:

The current output creates the release page using the default template. The output from the sample run is in the log and can be seen rendered in my test repository.

Modifying the generated page is now easy (we just need a new template) and is not the focus of this PR.

- UpdateGitHubPage() is now implemented in `krel release`.
- The announce package now renders the GitHub page from the template before performing checks in the repository to be able to run while the release tag has not been created.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/release-eng Issues or PRs related to the Release Engineering subproject labels Nov 25, 2020
@k8s-ci-robot k8s-ci-robot added sig/release Categorizes an issue or PR as relevant to SIG Release. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 25, 2020
@puerco puerco mentioned this pull request Nov 25, 2020
Closed
18 tasks
@puerco
Copy link
Member Author

puerco commented Nov 25, 2020

Test Run

✔️ Sample release run: https://console.cloud.google.com/cloud-build/builds/a611426e-a163-4679-acb0-a2fcf596b87f?authuser=1&project=kubernetes-release-test

The github page step begins at line 1101

saschagrunert
saschagrunert approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 25, 2020
saschagrunert
saschagrunert reviewed Nov 25, 2020
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just left my two nits since this needs now a rebase. 👇

pkg/anago/release.go Outdated
Comment on lines 487 to 494
Owner: "kubernetes",
Repo: "kubernetes",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Owner: "kubernetes",
Repo: "kubernetes",
Owner: git.DefaultGithubOrg,
Repo: git.DefaultGithubRepo,

Copy link
Member Author

@puerco puerco Nov 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ done.

Do you think that later we should have these values in anago or maybe in release?

While I was considering using them yesterday it seemed to me that while having them in git seems like a good failsafe/sane default, if we see the git package in as more general purpose tool perhaps the release process should have it's own constants, decoupled from the git pkg defaults.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did the same thing in the release notes source btw ;)
I'll fix it at some point.

@saschagrunert saschagrunert mentioned this pull request Nov 25, 2020
Merged
@puerco puerco force-pushed the integrate-github-page branch from 8830ca7 to c1b6e48 Compare November 25, 2020 14:37
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 25, 2020
@puerco
Update the GitHub release page from krel release
21410f8 
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
@puerco
Process GitHub page before checking repo
b73f509 
We now check assets and build the github page before
checking the repository tags and releases to be able
to run in mock mode, before creating the tag.

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
@puerco puerco force-pushed the integrate-github-page branch from c1b6e48 to b73f509 Compare November 25, 2020 14:41
saschagrunert
saschagrunert approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

xmudrii
xmudrii approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 25, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: puerco, saschagrunert, xmudrii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hasheddan
hasheddan reviewed Nov 25, 2020
View reviewed changes
pkg/anago/release.go
Draft: false,
Owner: git.DefaultGithubOrg,
Repo: git.DefaultGithubRepo,
// PageTemplate: , // If we use a custom template, define it here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we mean to leave this comment in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes I left it intentionally as a guide when we decide what should become of the page (ref this in slack)
But I can take it out if it's better :)

@k8s-ci-robot k8s-ci-robot merged commit f77005b into kubernetes:master Nov 25, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Nov 25, 2020
@puerco puerco deleted the integrate-github-page branch November 27, 2020 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hasheddan hasheddan hasheddan left review comments

@xmudrii xmudrii xmudrii approved these changes

@saschagrunert saschagrunert saschagrunert approved these changes

Assignees

@saschagrunert saschagrunert

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.20
Development

Successfully merging this pull request may close these issues.
5 participants
@puerco @k8s-ci-robot @saschagrunert @xmudrii @hasheddan