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

updating resolvelib version to include new reporter interface #10600

Merged
merged 2 commits into from
Oct 21, 2021

Conversation

nadavwe
Copy link
Contributor

@nadavwe nadavwe commented Oct 19, 2021

a prerequisite for #10258

@uranusjr
Copy link
Member

Needs a news entry for the vendor update, otherwise lgtm.

@pradyunsg
Copy link
Member

We should update the debugging reporter as well.

@uranusjr
Copy link
Member

I think the idea is to only update vendoring in this PR and do all the pip changes in #10258 (that PR does change the debugging reporter).

@pradyunsg
Copy link
Member

Ah, cool. Ignore me -- I'm just being noisy then. :)

@nadavwe nadavwe force-pushed the vendoring-new-resolve-lib-version branch from 2f25bb3 to 93f5bf7 Compare October 21, 2021 06:22
@nadavwe
Copy link
Contributor Author

nadavwe commented Oct 21, 2021

Needs a news entry for the vendor update, otherwise lgtm.

I kinda knew I should add that... but wasn't sure what to write. I should have looked into another vendor upgrade before submitting.
How would you feel if I open a PR for _vendor/README.rst to include a paragraph on the news, so other people can just follow that readme when upgrading?

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Don't need to update README, this is fine.

@nadavwe
Copy link
Contributor Author

nadavwe commented Oct 21, 2021

How would you feel if I open a PR for _vendor/README.rst to include a paragraph on the news, so other people can just follow that readme when upgrading?

@uranusjr I think I did not make myself clear on this point. ⬆️
I did not mean to add news on the upgrade of resolvelib in _vendor/README.rst. (that, of course, would be quite silly).
I meant adding a section like:
"you should add a news piece when syncing vendored packages. it should look something like..." etc.

@uranusjr
Copy link
Member

This is already mentioned in the documentation:

https://pip.pypa.io/en/stable/development/contributing/#choosing-the-type-of-news-entry

Which I think is more visible than pip/_vendor/README.rst. But I guess it wouldn't hurt adding a line there as well to tell the contributor to refer to documentation.

@uranusjr
Copy link
Member

Anyway, this should be good since the static checks passed. Let's deal with this in another issue or PR.

@uranusjr uranusjr merged commit 98d9fdf into pypa:main Oct 21, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants