-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Updated logging.py for #9146 #9149
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
Conversation
Updated logging.py for pytest-dev#9146
Updated logging.py for pytest-dev#9146
Create 9146.doc.rst
Updated AUTHORS and added changelog file
for more information, see https://pre-commit.ci
Tip: you can add |
Thanks! But I see there are some checks which I see failed for eg. At least 1 approving review is required by reviewers with write access. What should I do in this case? Should I update the PR or let it be? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Vijay-Arora! Left a few request for changes, please take a look.
cc @MetRonnie
No change required on your side, this just means at least one maintainer needs to approve the PR. |
Hey @nicoddemus, should I go ahead and make the requested changes and open a new PR for this? Also, I see some failures, do I need to make some changes for that too? |
Just push new commits to the same branch and the PR will update automatically.
Hmm ignore them for now, they don't seem related to your PR, I will investigate those later. |
Cool, thanks, Making the changes now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on! As the author of the original issue, I've added my thoughts
@nicoddemus I do wonder if the docstring for |
@MetRonnie Good catch. As far as I understand it, the behavior is the same for @Vijay-Arora Would you be interested in driving this one home? 😄 |
Co-authored-by: Ronnie Dutta <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get this merged!
Close #9146