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.

Sign up for GitHub

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

Jump to bottom

Revert fspath deprecation #8903

Merged
merged 1 commit into from
Jul 14, 2021
Merged

Revert fspath deprecation #8903

merged 1 commit into from
Jul 14, 2021

Conversation

nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Jul 13, 2021 •
edited
Loading

It is not clear yet how we should proceed with this deprecation
because pytest.Item.reportinfo is public API and returns a py.path object,
and also how plugins and our examples should handle that.

Reverting just the deprecation aspect of #8251 so we can get a 7.0.0 release out.

We will reintroduce the deprecation later once we have a clear path moving forward with replacing reportinfo.

Closes #8445
Closes #8821

@nicoddemus
Copy link
Member Author

As per #8821 (comment), decided to go ahead with a PR, we can accept or decline it here.

If accepted, I will create a follow up issue to reintroduce the deprecation at a later date.

@nicoddemus
Revert fspath deprecation
2c4c57e 
It is not clear yet how we should proceed with this deprecation
because `pytest.Item.reportinfo` is public API and returns a `py.path` object,
and is not clear how plugins and our examples should handle that.

Reverting just the deprecation aspect of pytest-dev#8251 so we can get a 7.0.0 release out.

We will reintroduce the deprecation later once we have a clear path moving forward with replacing `reportinfo`.

Closes pytest-dev#8445
Closes pytest-dev#8821
@nicoddemus nicoddemus force-pushed the remove-fspath-deprecation branch from 4a9dad8 to 2c4c57e Compare July 13, 2021 11:24
The-Compiler
The-Compiler approved these changes Jul 13, 2021
View reviewed changes
Copy link
Member

@The-Compiler The-Compiler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up! Seems reasonable, for context, the original deprecation was in #8251.

I agree with what @bluetech has said in #8251 (comment) - let's release 7.0 without this deprecation, and then try to do a proper cut, probably for 8.0.

@The-Compiler
Copy link
Member

Oh, I see you're also closing #8445 from this. However, what about HOOK_LEGACY_PATH_ARG introduced in #8463 (as follow-up to #8144)? Do we want to keep that deprecation or should it be reverted as well? If we keep it, do we have enough docs for it (I think we do)?

@nicoddemus
Copy link
Member Author

If we keep it, do we have enough docs for it (I think we do)?

I think the docs are good as they are, because the upgrade path is clear.

Node.fspath is different because one can't reasonably implement the public reportinfo API without hacks, so better do postpone it.

@nicoddemus nicoddemus merged commit 69356d2 into pytest-dev:main Jul 14, 2021
@nicoddemus nicoddemus deleted the remove-fspath-deprecation branch July 14, 2021 12:32
@nicoddemus nicoddemus mentioned this pull request Jul 14, 2021
Closed
@nicoddemus
Copy link
Member Author

nicoddemus commented Jul 14, 2021 •
edited
Loading

Created follow up issue: #8908

@The-Compiler The-Compiler mentioned this pull request Nov 8, 2021
Closed
The-Compiler added a commit to The-Compiler/pytest that referenced this pull request Nov 8, 2021
@The-Compiler
Remove changelog entry for pytest-dev#8251
5c6d87b 
Reverted in pytest-dev#8903
The-Compiler added a commit that referenced this pull request Nov 10, 2021
@The-Compiler
More pytest 7 changelog fixes (#9285)
f9afcd2 
* Remove changelog entry for #8251

Reverted in #8903

* Move #9202 changelog to to trivial

This won't concern users of pytest

* Streamline deprecation changelogs/docs

* Remove #8994 changelog

This is an impovement for a warning introduced in this release, so including it in a changelog against the last release seems confusing.

* Remove #9241 changelog

This is an impovement for a doc update introduced in this release, so including it in a changelog against the last release seems confusing. The issue number also seems about something different.

* Remove #8897 changelog

Empty file...

* Various minor changelog fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@The-Compiler The-Compiler The-Compiler approved these changes

@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix fspath warnings in nonpython.rst example add more docs on the fspath deprecation
3 participants
@nicoddemus @The-Compiler @RonnyPfannschmidt