🐛 Fix wrong branch name display for weekly update script #9918

chandankumar4 · 2023-12-22T17:15:28Z

What this PR does / why we need it:
Fix the branch name display issue after running the command make release-weekly-update-tool from branch other than main.

Example: Output of command after bug fix!!

$:: github.com/sigs.k8s.io/cluster-api ‹release-1.6*› » ./bin/weekly --from 2023-12-11 --to 2023-12-17 --milestone v1.7
Weekly update :rotating_light:
Changes from 2023-12-11 to 2023-12-17 a total of 7 new commits where merged into release-1.6.

- 1 :sparkles: New Features
         - KCP: Allow mutation of all fields that should be mutable (#9885)

- 4 :bug: Bug Fixes
         - capd: fix ignition to also set the kube-proxy configuration to skip setting sysctls (#9895)
         - Improve log k/v pairs and a improve/drop a few log lines (#9880)
         - ipam: fix webhooks using mixed api versions (#9863)
         - clusterctl: validate no objects exist from CRDs before deleting them (#9834)

- 2 Other changes :seedling:

All merged PRs can be viewed in GitHub:
https://github.com/kubernetes-sigs/cluster-api/pulls?q=is%3Apr+closed%3A2023-12-11..2023-12-18+is%3Amerged+milestone%3Av1.7+

_Thanks to all our contributors!_ 😊
/Your friendly comms release team

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes # N/A

/area release

k8s-ci-robot · 2023-12-22T17:15:36Z

Welcome @chandankumar4!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-12-22T17:15:37Z

Hi @chandankumar4. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mcbenjemaa · 2023-12-22T17:56:35Z

/ok-to-test

willie-yao

Looks good pending one minor comment. Thanks for working on this!

hack/tools/release/weekly/main.go

willie-yao

/lgtm

k8s-ci-robot · 2023-12-22T19:55:15Z

LGTM label has been added.

Git tree hash: 3c4984f75560f25a0bbc453132d682e7dbfe66ad

chandankumar4 · 2024-01-03T17:48:11Z

Hey @mcbenjemaa @willie-yao if you get time, can you please take a look once, Is there any more update I need to do in this PR?

willie-yao · 2024-01-08T21:55:32Z

This looks good to me. Just waiting on an approver to take a look. cc @sbueringer

sbueringer · 2024-01-09T09:45:59Z

/assign @cahillsf @g-gaston

cahillsf · 2024-01-09T14:02:38Z

hack/tools/release/weekly/main.go

+ // fetch the current branch
+ out, err = exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD").CombinedOutput()
+ if err != nil {
+ fmt.Println("Error")


this is a bit confusing, i think we can consolidate the "Error" string to print in the same line as err

I have followed the same pattern which is there in line no: 105 after executing any command. Do I need to update in all places?

ah cool, if this is a pattern that works for the comms team it works for me 👍

cahillsf · 2024-01-09T14:06:23Z

hack/tools/release/weekly/main.go

+ if err != nil {
+ fmt.Println("Error")
+ fmt.Println(err)
+ fmt.Println(string(out))


is this a debug print statement? if there is an error in attempting to assign the value and we are about to return 1, why print this?

Similar comment as above.

cahillsf · 2024-01-09T17:55:58Z

question on your example output, is it expected to have the milestone minor version differ from the branch minor version?
$:: github.com/sigs.k8s.io/cluster-api ‹release-1.6*› » ./bin/weekly --from 2023-12-11 --to 2023-12-17 --milestone v1.7

the corresponding docs don't really specify:

cluster-api/docs/release/release-tasks.md

Line 338 in c8c0794

#### Generate weekly PR updates to post in Slack

chandankumar4 · 2024-01-12T08:07:21Z

from the branch minor version

@cahillsf
Yes, that's the expected behaviour, the use of milestone minor version is for targeting against which version the notes should be generated and branch minor version is indicating that from which branch notes is generated.
cc @willie-yao

cahillsf · 2024-01-12T14:10:22Z

from the branch minor version

@cahillsf Yes, that's the expected behaviour, the use of milestone minor version is for targeting against which version the notes should be generated and branch minor version is indicating that from which branch notes is generated. cc @willie-yao

hm, that seems at odds with the most recent update provided in the CAPI channel: https://kubernetes.slack.com/archives/C8TSNPY4T/p1704956238693459?thread_ts=1704956148.936569&cid=C8TSNPY4T

the current branch and the milestone are the same:

Changes from 2024-01-01 to 2024-01-07 a total of 1 new commits where merged into release-1.5.
1 Other changes 🌱
All merged PRs can be viewed in GitHub:
https://github.com/kubernetes-sigs/cluster-api/pulls?q=is%3Apr+closed%3A2024-01-01..2024-01-08+is%3Amerged+milestone%3Arelease-1.5+

chandankumar4 · 2024-01-15T18:20:04Z

@cahillsf In this link: https://kubernetes.slack.com/archives/C8TSNPY4T/p1704956238693459?thread_ts=1704956148.936569&cid=C8TSNPY4T milestone is v1.7 and current branch is release-1.5

cahillsf · 2024-01-15T20:45:05Z

@cahillsf In this link: https://kubernetes.slack.com/archives/C8TSNPY4T/p1704956238693459?thread_ts=1704956148.936569&cid=C8TSNPY4T milestone is v1.7 and current branch is release-1.5

followed up in that thread, but those links were actually not correct. this is actually not related to your pull request, but i believe the docs need to be updated (or the behavior of the tool needs to be improved). here is the link in your example: https://github.com/kubernetes-sigs/cluster-api/pulls?q=is%3Apr+closed%3A2023-12-11..2023-12-18+is%3Amerged+milestone%3Av1.7+

you will see that the commits shown do not correspond to what the generated output message says, , i will open a separate issue regarding the links/doc

thanks for working on this improvement. lgtm pending squashed commits

Signed-off-by: chandankumar4 <[email protected]>

cahillsf · 2024-01-16T13:22:09Z

/lgtm
/approve
/hold (feel free to unhold, not sure if you want more reviews)

k8s-ci-robot · 2024-01-16T13:22:14Z

LGTM label has been added.

Git tree hash: 1c268924b488bac0ce57288505e0c8ce421c0053

k8s-ci-robot · 2024-01-16T13:22:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cahillsf

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:

~~hack/tools/release/OWNERS~~ [cahillsf]

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

chandankumar4 · 2024-01-16T17:20:06Z

/unhold

chandankumar4 · 2024-01-16T17:21:47Z

Have checked with @willie-yao and he is good with this change!!

k8s-ci-robot added area/release Issues or PRs related to releasing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 22, 2023

k8s-ci-robot requested review from mcbenjemaa and VibhorChinda December 22, 2023 17:15

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 22, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 22, 2023

willie-yao reviewed Dec 22, 2023

View reviewed changes

hack/tools/release/weekly/main.go Outdated Show resolved Hide resolved

chandankumar4 force-pushed the fix-weekly-update-branch branch from 50ee7dd to a4bd4f5 Compare December 22, 2023 18:46

willie-yao approved these changes Dec 22, 2023

View reviewed changes

k8s-ci-robot assigned willie-yao Dec 22, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 22, 2023

k8s-ci-robot assigned cahillsf and g-gaston Jan 9, 2024

cahillsf reviewed Jan 9, 2024

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 12, 2024

k8s-ci-robot requested review from cahillsf and g-gaston January 12, 2024 08:10

k8s-ci-robot requested a review from willie-yao January 12, 2024 08:10

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2024

cahillsf mentioned this pull request Jan 15, 2024

Improve Generate weekly PR updates to post in Slack docs/process #10008

Closed

Fix wrong branch name display for weekly update script

bfe8d0d

Signed-off-by: chandankumar4 <[email protected]>

chandankumar4 force-pushed the fix-weekly-update-branch branch from 9b340b3 to bfe8d0d Compare January 16, 2024 06:49

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 16, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 16, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 16, 2024

k8s-ci-robot merged commit 4216a96 into kubernetes-sigs:main Jan 16, 2024
21 checks passed

k8s-ci-robot added this to the v1.7 milestone Jan 16, 2024

chandankumar4 deleted the fix-weekly-update-branch branch January 16, 2024 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Fix wrong branch name display for weekly update script #9918

🐛 Fix wrong branch name display for weekly update script #9918

chandankumar4 commented Dec 22, 2023

k8s-ci-robot commented Dec 22, 2023

k8s-ci-robot commented Dec 22, 2023

mcbenjemaa commented Dec 22, 2023

willie-yao left a comment

willie-yao left a comment

k8s-ci-robot commented Dec 22, 2023

chandankumar4 commented Jan 3, 2024 •

edited

Loading

willie-yao commented Jan 8, 2024

sbueringer commented Jan 9, 2024

cahillsf Jan 9, 2024

chandankumar4 Jan 9, 2024 •

edited

Loading

cahillsf Jan 9, 2024

cahillsf Jan 9, 2024

chandankumar4 Jan 9, 2024

cahillsf commented Jan 9, 2024 •

edited

Loading

chandankumar4 commented Jan 12, 2024 •

edited

Loading

cahillsf commented Jan 12, 2024

chandankumar4 commented Jan 15, 2024

cahillsf commented Jan 15, 2024 •

edited

Loading

cahillsf commented Jan 16, 2024

k8s-ci-robot commented Jan 16, 2024

k8s-ci-robot commented Jan 16, 2024

chandankumar4 commented Jan 16, 2024

chandankumar4 commented Jan 16, 2024

🐛 Fix wrong branch name display for weekly update script #9918

🐛 Fix wrong branch name display for weekly update script #9918

Conversation

chandankumar4 commented Dec 22, 2023

k8s-ci-robot commented Dec 22, 2023

k8s-ci-robot commented Dec 22, 2023

mcbenjemaa commented Dec 22, 2023

willie-yao left a comment

Choose a reason for hiding this comment

willie-yao left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Dec 22, 2023

chandankumar4 commented Jan 3, 2024 • edited Loading

willie-yao commented Jan 8, 2024

sbueringer commented Jan 9, 2024

cahillsf Jan 9, 2024

Choose a reason for hiding this comment

chandankumar4 Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

cahillsf Jan 9, 2024

Choose a reason for hiding this comment

cahillsf Jan 9, 2024

Choose a reason for hiding this comment

chandankumar4 Jan 9, 2024

Choose a reason for hiding this comment

cahillsf commented Jan 9, 2024 • edited Loading

chandankumar4 commented Jan 12, 2024 • edited Loading

cahillsf commented Jan 12, 2024

chandankumar4 commented Jan 15, 2024

cahillsf commented Jan 15, 2024 • edited Loading

cahillsf commented Jan 16, 2024

k8s-ci-robot commented Jan 16, 2024

k8s-ci-robot commented Jan 16, 2024

chandankumar4 commented Jan 16, 2024

chandankumar4 commented Jan 16, 2024

chandankumar4 commented Jan 3, 2024 •

edited

Loading

chandankumar4 Jan 9, 2024 •

edited

Loading

cahillsf commented Jan 9, 2024 •

edited

Loading

chandankumar4 commented Jan 12, 2024 •

edited

Loading

cahillsf commented Jan 15, 2024 •

edited

Loading