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

Switch to using the standards format (requirements.txt) for Python deps. #21

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

sseering
Copy link
Contributor

@sseering sseering commented Nov 5, 2024

No description provided.

@flokoe flokoe assigned flokoe and sseering and unassigned flokoe Nov 7, 2024
@flokoe flokoe self-requested a review November 7, 2024 10:19
Copy link
Owner

@flokoe flokoe left a comment

Choose a reason for hiding this comment

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

Hi there!

Thanks for your contribution.

The requirements.txt is more a common way to express dependencies for pip (and pip only) than a real standard.

I am missing the "why" for this PR. What value does this change provide in contrast to the old variant? Currently, this PR just moves the location where dependencies are stored.

Maybe freeze the versions of the dependencies in the requirements.txt? This would add the value of locking the dependencies for everyone.

With locked dependencies, I would happily accept this PR.

@sseering
Copy link
Contributor Author

sseering commented Nov 7, 2024

The requirements.txt is more a common way to express dependencies for pip (and pip only) than a real standard.

I am missing the "why" for this PR. What value does this change provide in contrast to the old variant? Currently, this PR just moves the location where dependencies are stored.

Having the dependecies in a machine readable format is useful in the long term. That was the whole goal.

There might not be a a full agreement from all Pyhon tools to use the file. But its widely used enough and well specified enough for me: https://pip.pypa.io/en/stable/reference/requirements-file-format/

Maybe freeze the versions of the dependencies in the requirements.txt? This would add the value of locking the dependencies for everyone.

With locked dependencies, I would happily accept this PR.

I'm not familiar enough with the code to determine the min/max versions for dependencies.

Arguably, its not even a good idea to pin the versions of dependencies.

The current way of pip install mkdocs-material mkdocs-git-revision-date-localized-plugin mkdocs-awesome-pages-plugin mkdocs-minify-plugin mkdocs serve will always install the current stable versions of all dependencies. The requirements.txt suggestted by me keeps this behaviour.

From the Readme.md showing pip install mkdocs-material mkdocs-git-revision-date-localized-plugin mkdocs-awesome-pages-plugin mkdocs-minify-plugin mkdocs serve one can assume that this project is expected to always run with the current stable versions of all dependencies. This is a good goal and should not be changed. Pinning to versions that will eventually become outdated would be a downgrade.

@flokoe
Copy link
Owner

flokoe commented Nov 8, 2024

Arguably, its not even a good idea to pin the versions of dependencies.

I disagree.

The current way of pip install mkdocs-material mkdocs-git-revision-date-localized-plugin mkdocs-awesome-pages-plugin mkdocs-minify-plugin, mkdocs serve will always install the current stable versions of all dependencies. The requirements.txt suggestted by me keeps this behaviour.

This may be true and I agree that we should always use the latest stable software, but without pinning we are not able to guarantee this and people are notoriously bad at keeping their software up to date.

Pinning to versions that will eventually become outdated would be a downgrade.

This is pretty much a non-concern, as it is trivial to automate. I'll be happy to add Renovate Bot to this Repo. See #23

Now, that said, pinning versions for this project isn't that critical as we only consume software instead of creating it. But I still would like to add version pinning as it is good practice.

I'm not familiar enough with the code to determine the min/max versions for dependencies.

Currently, we could just use the latest versions as a starting point. As far as I know we do not use any special features. A fresh install and pip freeze would do the trick.

Copy link
Owner

@flokoe flokoe left a comment

Choose a reason for hiding this comment

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

I guess it does not hurt and it is an incremental step towards dependency pinning. I will accept your PR.

Thanks for your contribution!

@flokoe flokoe merged commit 5fdf713 into flokoe:main Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants