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

Remove mentions of "required settings" in for_metric args from docs #114

Closed
Sinjo opened this issue May 1, 2019 · 0 comments · Fixed by #120
Closed

Remove mentions of "required settings" in for_metric args from docs #114

Sinjo opened this issue May 1, 2019 · 0 comments · Fixed by #120
Milestone

Comments

@Sinjo
Copy link
Member

Sinjo commented May 1, 2019

Another piece of PR feedback. It's not something we use ourselves. It was there to give flexibility to future stores people might write.

I can see how it would play badly with pre-canned metrics exported by libraries. Let's take it out of the docs.

@Sinjo Sinjo added this to the multi-process milestone May 1, 2019
@Sinjo Sinjo changed the title Remove docs that mention "required" settings in for_metric Remove docs that mention "required settings" in for_metric May 1, 2019
@Sinjo Sinjo changed the title Remove docs that mention "required settings" in for_metric Remove mentions of "required settings" in for_metric args from docs May 1, 2019
Sinjo pushed a commit that referenced this issue May 3, 2019
  - Don't suggest defining metrics outside of file they're used in
  - Don't allow stores to require extra parameters in `for_metric`
  - Correct note on kernel page cache

Fixes #113, #114
Sinjo pushed a commit that referenced this issue May 3, 2019
  - Don't suggest defining metrics outside of file they're used in
  - Don't allow stores to require extra parameters in `for_metric`
  - Correct note on kernel page cache

Fixes #113, #114

Signed-off-by: Chris Sinjakli <chris@gocardless.com>
@Sinjo Sinjo closed this as completed in #120 May 5, 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 a pull request may close this issue.

1 participant