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.

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

Use head_branch metric #2549

Merged
merged 13 commits into from
May 28, 2023
Merged

Use head_branch metric #2549

merged 13 commits into from
May 28, 2023

Conversation

th-le
Copy link
Contributor

@th-le th-le commented Apr 28, 2023

Follow up for #2176.

This PR uses the branch_name for metrics

@th-le th-le changed the title Use branch_name metric Use head_branch metric May 1, 2023
@Link- Link- added needs triage Requires review from the maintainers community Community contribution labels May 1, 2023
@th-le
Copy link
Contributor Author

th-le commented May 9, 2023

@mumoshu could you help take a look

go.mod Outdated
@@ -60,6 +62,7 @@ require (
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/gnostic v0.5.7-v3refs // indirect
github.com/google/go-github/v45 v45.2.0 // indirect
github.com/google/go-github/v52 v52.0.0
Copy link
Collaborator

@mumoshu mumoshu May 27, 2023

Choose a reason for hiding this comment

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

Thank you @th-le! TIL that this had to be bumped in order to accommodate google/go-github@9385ab0.
However, I'm still unsure why this had to be in this second block of require.
Shouldn't it have resulted in updating L13 of this file?!

Comment on lines +28 to +29
golang.org/x/net v0.9.0
golang.org/x/oauth2 v0.7.0
Copy link
Collaborator

@mumoshu mumoshu May 27, 2023

Choose a reason for hiding this comment

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

These two versions seem to align with go-github v52 👍

https://github.com/google/go-github/blob/v52.0.0/go.mod

go.mod Outdated
golang.org/x/sync v0.1.0
gomodules.xyz/jsonpatch/v2 v2.2.0
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.26.2
k8s.io/apimachinery v0.26.2
k8s.io/client-go v0.26.2
sigs.k8s.io/controller-runtime v0.14.4
sigs.k8s.io/controller-runtime v0.14.6
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we leave this controller-runtime upgrade to another pull request to make this pull request focused on the head_branch addition? @th-le

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

I made changes that I thought necessary myself and merging this now!
It would be great if you could confirm when you have time ☺️
Thanks again for your contribution @th-le!!

@mumoshu mumoshu merged commit e7ec736 into actions:master May 28, 2023
14 checks passed
@th-le
Copy link
Contributor Author

th-le commented May 29, 2023

the changes look good. thank you so much @mumoshu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution needs triage Requires review from the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants