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

Backport Prometheus Server Metrics #1778

Merged
merged 5 commits into from Apr 11, 2018

Conversation

Projects
None yet
2 participants
@ChristopherDavenport
Member

ChristopherDavenport commented Apr 11, 2018

Would be cool to cut a release with this.

@rossabaker

I think this is a straight backport, so I won't die on this hill. Just noticed a couple things this time.

class PrometheusExportService[F[_]: Sync] private (
s: HttpService[F],
cr: CollectorRegistry

This comment has been minimized.

@rossabaker

rossabaker Apr 11, 2018

Member

If these are vals, you don't need the defs.

This could be a final class.

This comment has been minimized.

@ChristopherDavenport

ChristopherDavenport Apr 11, 2018

Member

I agree, but unless you want pain on the way forward, seems like that should be done after we have gotten this through.

edit: On second thought I am open to either way

@rossabaker

Nit: I liked the full names better than the parameters names. I should have been more clear.

ChristopherDavenport added some commits Apr 11, 2018

@ChristopherDavenport ChristopherDavenport merged commit e64a792 into http4s:release-0.18.x Apr 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ChristopherDavenport ChristopherDavenport deleted the ChristopherDavenport:backPortPrometheus branch Apr 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment