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

add FixtureManager to docs #3430

Closed
wants to merge 1 commit into from
Closed

add FixtureManager to docs #3430

wants to merge 1 commit into from

Conversation

svenevs
Copy link
Contributor

@svenevs svenevs commented Apr 25, 2018

Fixes #3420.

@nicoddemus encouraged me to submit a PR, but please understand I have not been able to render the docs locally to verify that this documentation links / finds the class correctly (error in third party docs extension).

Commits from maintainers can be done on that fork, as well as I'm happy to move it to another location on the docs etc. Thanks for pytest, the learning curve was totally worth it ❤️

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 92.856% when pulling f5a19bc on svenevs:master into 0b2b73c on pytest-dev:master.

@nicoddemus
Copy link
Member

Thanks a lot for following up with the PR @svenevs, we appreciate it!

Glad to know you are enjoying working/learning pytest! 👍

@nicoddemus
Copy link
Member

Uh oh, sorry to only realize this now: the FixtureManager is not really part of the public API, so it should not go to the docs. The reference docs is meant only for public API the user calls directly or is accessible from public objects, but the FixtureManager is an implementation detail that we reserve the right to change in the future as part of an internal refactoring or implementation of a new feature.

I'm really sorry I answered to your question in #3420 (comment) in haste and made you waste your time preparing this PR. 😞

Again thanks for the PR, totally my fault for suggesting to write a PR, but please do stick around to work on something else if you feel like. Thanks for understanding!

@svenevs
Copy link
Contributor Author

svenevs commented Apr 25, 2018

No problem, creation of the PR took maybe 5 minutes since I saved the diff 😄

@nicoddemus
Copy link
Member

Hehehe great, thanks!

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