-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Architecture Documentation: Configuration deep dive #7858
Architecture Documentation: Configuration deep dive #7858
Conversation
This section of the documentation is currently being written. pip | ||
developers welcome your help to complete this documentation. If you're | ||
interested in helping out, please let us know in the | ||
`tracking issue <https://github.com/pypa/pip/issues/6831>`_. |
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.
I see this is being repeated in #7859. Would it make sense to make this a custom derivative? This can be useful for other parts of the documentation as well, if the tracking issue URL is made variable.
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.
Yup - that's an excellent idea. I'll do it in a separate PR tho.
@leonardr thank you for agreeing to help me out here. This document, when finished enough for publication, will live within pip's developer/contributor docs and help contributors understand how pip handles users' configuration files (such as these). @pradyunsg said elsewhere:
Leonard, if you could help give @pradyunsg some feedback, that would be great. You see he has a "TODO: Figure out how to structure the initial part of this document." and that's the most crucial guidance he needs right 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.
With the caveat that I don't know anything about the underlying code--I've only read the existing outline--here's my suggestion:
When documenting a class I usually talk about the typical lifecycle of an instance. In this case Configuration
seems to be heavily associated with the pip
command. Of course you can create a Configuration
from scratch but I don't imagine a lot of people would do that. So this example talks about pip
and pip config
as entry points to the Python APIs that are actually taking up most of the documentation.
During the startup phase of the
pip
command, aConfiguration
object is typically created from the local environment: some combination of config files and environment variables. ThisConfiguration
object provides a Python API for reading and changing the configuration values.A
Configuration
object also has an API for writing the consolidated configuration values back to a single config file. Thepip config
subcommand uses this to provide a command-line interface for manipulating configuration.
a5d914b
to
8eb3330
Compare
Okay, I think this is ready for review now! :) The newly added document, built by an RTD beta service is available for preview at: https://pip--7858.org.readthedocs.build/en/7858/development/architecture/configuration-files/ |
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.
Some comments and suggestions, but overall this looks good 🙂
sigh |
There seem to be network issues with the tests on Azure (and sometimes travis as well). Not sure what's going on, master also failed on me earlier 🙁 |
The errors are PyPI-side: https://twitter.com/jezdez/status/1243130982900862978 |
Pinging @ewdurbin, in case (somehow) he hasn't noticed the errors yet. |
I believe PyPI is now back up. |
This is good to go. I hope to merge this in ~12-24 hours. Reviews (even post-merge) are welcome and we can iterate on this as we go as well. :) |
a817642
to
98f1a95
Compare
Merging this now. Post-merge reviews are welcome (until @lock[bot] locks this) and I'm more than happy to iterate on this. Thanks for the inputs @brainwane @leonardr @uranusjr @pfmoore! Much appreciated! ^>^ |
Toward #6831