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

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

Merged
merged 1 commit into from
Jan 16, 2024
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion hack/tools/release/weekly/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,24 @@ func run() int {
merges[key] = append(merges[key], formatMerge(body, prNumber))
}

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@chandankumar4 chandankumar4 Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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

fmt.Println(err)
fmt.Println(string(out))
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar comment as above.

return 1
}

branch := strings.TrimSpace(string(out))
if branch == "" {
fmt.Println("Error: failed to get current branch!!!")
return 1
}

// TODO Turn this into a link (requires knowing the project name + organization)
fmt.Println("Weekly update :rotating_light:")
fmt.Printf("Changes from %v a total of %d new commits were merged into main.\n\n", commitRange, len(commits))
fmt.Printf("Changes from %v a total of %d new commits were merged into %s.\n\n", commitRange, len(commits), branch)

for _, key := range outputOrder {
mergeslice := merges[key]
Expand Down