Skip to content

Update example README for #111 - #126

Merged
Sinjo merged 1 commit into
prometheus:masterfrom
swalberg:patch-1
May 31, 2019
Merged

Sinjo merged 1 commit into
prometheus:masterfrom
swalberg:patch-1

Conversation

@swalberg

Copy link
Copy Markdown
Contributor

I was trying to modify the path that's exported and didn't realize it was just changed. The documentation in the example project still reflects the old way.

Gave a short example of how I did it -- I realize subclassing a protected method is dangerous but there are no other hooks.

FWIW my use case was that the default regular expressions did not cover my url structure and I needed to add another one. I did not need to add more labels.

@coveralls

coveralls commented May 27, 2019 •

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 661fa88 on swalberg:patch-1 into 982fe2e on prometheus:master.

I was trying to modify the path that's exported and didn't realize it was _just_ changed. The documentation in the example project still reflects the old way.

Gave a short example of how I did it -- I realize subclassing a protected method is dangerous but there are no other hooks.

FWIW my use case was that the default regular expressions did not cover my url structure and I needed to add another one. I did not need to add more labels.

Signed-off-by: Sean Walberg <sean@ertw.com>
@Sinjo

Sinjo commented May 31, 2019

Copy link
Copy Markdown
Member

Hi @swalberg,

Thanks for taking time to write this and send it over!

I'm going to merge as is and make a couple of tweaks to caveat that extending the class isn't something we support as an API, but that it is an option.

To give a bit of extra clarity on the Rack middleware situation: making it work with the the up-front label declaration left us with an API we thought was more confusing than it was worth.

I am keen to come back to it into the future, but for now I'm focusing on the road to 1.0, which means focusing on more breaking changes like #107 (which I'm working on today).

@Sinjo
Sinjo merged commit 710579c into prometheus:master May 31, 2019
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.

3 participants