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

Bump Prometheus library #542

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Conversation

SuperQ
Copy link
Contributor

@SuperQ SuperQ commented Jul 7, 2021

Update Prometheus client library to the latest release.

Signed-off-by: SuperQ [email protected]

Update Prometheus client library to the latest release.

Signed-off-by: SuperQ <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2021

CLA assistant check
All committers have signed the CLA.

@banaag banaag requested a review from MichaelRybak July 7, 2021 16:38
@SuperQ
Copy link
Contributor Author

SuperQ commented Jul 7, 2021

Ugh, more CLAs

@MichaelRybak
Copy link
Contributor

Thank you Ben.

@twifkak
Copy link
Member

twifkak commented Jul 7, 2021

@SuperQ Thanks for the fix! Looks like it's blocked on CLA. Is the bot busted or did you not sign? LMK if you'd prefer we submit our own fix.

@SuperQ
Copy link
Contributor Author

SuperQ commented Jul 8, 2021

I had to find the time outside of work to read through the agreement. It seems reasonable, so I have signed it.

It's far easier to use work time for projects that use a standard Linux foundation DCO. Having a CLA means if I want to use work time for a project, I have to run every single one through our lawyers.

@twifkak
Copy link
Member

twifkak commented Jul 8, 2021

@SuperQ Frustration understood. I just wanted to make sure our bot wasn't stuck (and the offer to take over the PR was genuine). The choice of CLA or DCO is mostly beyond my influence (or even knowledge), but if you're curious, I recall that a discussion took place on ampproject/meta-tsc#25.

Anyway, I'll merge now, thanks again!

@twifkak twifkak merged commit 88f0d3c into ampproject:main Jul 8, 2021
@SuperQ SuperQ deleted the superq/prom_metrics branch July 8, 2021 22:09
@SuperQ
Copy link
Contributor Author

SuperQ commented Jul 9, 2021

Thanks!

One thing I noticed is that the default branch for this project isn't main, should I have made this PR against releases? Or do I need to make a separate PR for that? It seems like main is a bit out of sync.

@twifkak
Copy link
Member

twifkak commented Jul 9, 2021

main is correct; it's the dev branch. No need to do anything else unless it's worth a cherry-pick. main gets merged into releases every ~2 months after integration testing (with the AMP validator and with Google-internal SXG validation code).

[Side note: It is confusing that releases is the default branch; I'd like to change that. We chose this layout because go get doesn't support support downloading non-default branches (source). But maybe the new module-aware go install does? If so, it may be worth dropping support for older versions of Go.]

@SuperQ
Copy link
Contributor Author

SuperQ commented Jul 9, 2021

Yes, if you make the release tags semver you can use go install github.com/ampproject/[email protected]

You can also use a latest string there and it will pick the highest semver.

But I would stay at v0.1.0 or v1.0.0 for the new tags.

@twifkak
Copy link
Member

twifkak commented Jul 9, 2021

Cool, thanks for the tip. Filed #544 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants