Skip to content

Comments

fix #10342: put location into warning exceptions#10343

Merged
RonnyPfannschmidt merged 2 commits intopytest-dev:mainfrom
RonnyPfannschmidt:fix-10342-warn-explicit-add-location
Oct 7, 2022
Merged

fix #10342: put location into warning exceptions#10343
RonnyPfannschmidt merged 2 commits intopytest-dev:mainfrom
RonnyPfannschmidt:fix-10342-warn-explicit-add-location

Conversation

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Oct 6, 2022

as the warning systems own warn_explicit looses the information we add them explicitly to the warning exceptions

@The-Compiler this should resolve the issue you noted

closes #10342

@RonnyPfannschmidt
fix pytest-dev#10342: put location into warning exceptions
14919c4 
as the warning systems own warn_explicit looses the information
we add them explicitly to the warning exceptions
nicoddemus
nicoddemus approved these changes Oct 6, 2022
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

I suppose this also needs a changelog, as improvement?

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Oct 6, 2022

It's unreleased, so I consider it part of the initial change

@nicoddemus
Copy link
Member

nicoddemus commented Oct 6, 2022

Fair enough, did not realize that. 👍

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Oct 6, 2022

@The-Compiler i'd like to get your input before merging

@The-Compiler
Copy link
Member

The-Compiler commented Oct 7, 2022

I wanted to try this by installing git+https://github.com/RonnyPfannschmidt/pytest.git@fix-10342-warn-explicit-add-location, but I get:

The conflict is caused by:
    The user requested pytest 5.3.1.dev3295+g14919c4b (from git+https://github.com/RonnyPfannschmidt/pytest.git@fix-10342-warn-explicit-add-location)
    pytest-bdd 6.0.1 depends on pytest>=6.2.0

Is your branch outdated or something, or is that just a setuptools-scm thing?

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Oct 7, 2022

@The-Compiler i believe that i didn't push the tags to my repo, let me fix

@RonnyPfannschmidt
Copy link
Member Author

RonnyPfannschmidt commented Oct 7, 2022

@The-Compiler updated

The-Compiler
The-Compiler approved these changes Oct 7, 2022
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.

Great idea with only adding it if warnings are exceptions! Only a small suggestion, but looks good to go.

@RonnyPfannschmidt RonnyPfannschmidt merged commit 24ef7c9 into pytest-dev:main Oct 7, 2022
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

@nicoddemus nicoddemus nicoddemus 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.

Old-style hookimpl warning has no location information

3 participants

@RonnyPfannschmidt @nicoddemus @The-Compiler