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

Fix POETRY_REPOSITORIES_ variable docs #8492

Merged

Conversation

SamambaMan
Copy link
Contributor

POETRY_REPOSITORIES__URL wrong in the documentation

Pull Request Check List

Resolves: #8491

No changes are required to the code.

@dimbleby
Copy link
Contributor

it's not so simple as that: POETRY_REPOSITORIES_<NAME>_URL is specifically for setting the url associated with a repository ie it corresponds to repositories.<name>.url.

I can believe that there are clarity improvements to be made here and it would be good to see that: but I don't think this MR as it stands helps with that

@SamambaMan
Copy link
Contributor Author

What do you suggest?
I merely wrote the same as it in the code (i think) for doing as the manual won't work.

Do you want me to explain that the should match naming in sources?

@dimbleby
Copy link
Contributor

the current doc has a heading repositories.name. You are reporting - I think rightly - that as an environment variable this isn't very useful because you never want to configure only a repository name: you want to configure sub-properties of the repository (in particular, the URL).

However your MR leaves that heading alone, so now the doc is not self-consistent.

I realise that disentangling this is more than the four character fix you were hoping for!

@SamambaMan
Copy link
Contributor Author

Oh, i get it now.
This should cover other parameters as priority as well, right?
I'm hovering the code and this is not supported code wise yet also, right?

@dimbleby
Copy link
Contributor

As I understand it, global repository configuration is only interesting during publishing, so eg priority is irrelevant - I think url is the only interesting property.

@SamambaMan
Copy link
Contributor Author

So...? :D

@dimbleby
Copy link
Contributor

So: put some or more of that in the docs!

@SamambaMan
Copy link
Contributor Author

@dimbleby check if this is better.
Also included fixes for basic auth as tested in

monkeypatch.setenv("POETRY_HTTP_BASIC_FOO_USERNAME", "bar")

@SamambaMan
Copy link
Contributor Author

Can I improve this PR in any way?
Let me know

@radoering radoering added the impact/docs Contains or requires documentation changes label Oct 13, 2023
@github-actions
Copy link

github-actions bot commented Oct 13, 2023

Deploy preview for website ready!

✅ Preview
https://website-le50cq7z1-python-poetry.vercel.app

Built with commit 7bb092c.
This pull request is being automatically deployed with vercel-action

docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
SamambaMan and others added 4 commits October 25, 2023 06:49
@radoering radoering changed the title Fix POETRY_REPOSITORIES_ variable Fix POETRY_REPOSITORIES_ variable docs Oct 25, 2023
@radoering radoering merged commit 4bf1df5 into python-poetry:master Oct 25, 2023
18 checks passed
@SamambaMan SamambaMan deleted the fix_docs_repository_variable branch October 25, 2023 07:28
MrGreenTea pushed a commit to MrGreenTea/poetry that referenced this pull request Dec 18, 2023
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for POETRY_REPOSITORIES_<NAME> missing _URL suffix
3 participants