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

question: is there a reason why this repo doesn't have poetry.lock checked in? #77

Open
stupoid opened this issue Jul 2, 2021 · 6 comments

Comments

@stupoid
Copy link
Contributor

stupoid commented Jul 2, 2021

Just wondering if these were left out on purpose 🤔

@dimaqq
Copy link
Contributor

dimaqq commented Jul 5, 2021

re: .gitignore PRs are welcome :)

re: poetry.lock, two schools of thought are available:

  1. Only applications are allowed to pin dependencies, libraries can only include subdependency specifiers where it's important, like yarl = "^1.4.2" or, better yet yarn >= "1". An application may require a transient dependency via two or more libraries, e.g. app → aiodynamo → yarl; app → foobar → yarl, and there can only be one version installed in Python's site-packages (as opposed to node_modules). Additionally, leaving poetry.lock out kind-of-forces CI to test this library against newest versions, and library maintainer would get notified when subdependency becomes no longer compatible [in an ideal world]. Thus, @ojii believes it's better not to check poetry.lock into the library repository.

  2. When a library is added to an application, subdependency requirements from pyproject.toml will be used, and poetry.lock will be ignored, even if it were distributed. A source dist, .tar.gz includes pyproject.toml verbatim; A wheel .whl includes unpacks requirements into Requires-Dict: in .dist-info/METADATA. Either way, the information contained in poetry.lock is not distributed. Meanwhile, poetry.lock would work a specification for testing and local development, it would say that these specific subdependency versions and hashes were tested against and are guaranteed to work. Also they are safe to run in CI [assuming someone checked that]. Thus, @dimaqq would rather check poetry.lock in.

What do you think?

@ojii
Copy link
Contributor

ojii commented Jul 5, 2021

.gitignore because my local .gitignore starts with .* so it excluded itself and I didn't notice...

poetry.lock not included on purpose (see dimaqqs comment), in my opinion libraries should not use lock files.

@stupoid
Copy link
Contributor Author

stupoid commented Jul 5, 2021

@dimaqq regarding poetry.lock I'm in camp 2 for this one. Reproducible dev/test environment is always a nice thing to have.

Additionally, leaving poetry.lock out kind-of-forces CI to test this library against newest versions, and library maintainer would get notified when subdependency becomes no longer compatible [in an ideal world].

I'm not 100% certain, but I think dependabot does this even with poetry.lock checked in.

@ojii
Copy link
Contributor

ojii commented Jul 6, 2021

Reproducible dev/test environment is always a nice thing to have.

That sounds nice, but is it that useful for a library? Because what you get from pip install aiodynamo will be very different from what CI/dev envs run, unless the dependency selectors in pyproject.toml get restricted to whatever is in poetry.lock, which would make the library unusable. CI/dev should install the latest versions allowed by pyproject.toml since that's what's likely installed when you run pip install.

@dimaqq
Copy link
Contributor

dimaqq commented Aug 24, 2021

I'll add .gitignore now.

Another argument in favour of checked-in poetry.lock:
without it, poetry install is much slower, spending extra time here: Resolving dependencies... (56.6s)

Granted, that's hit only rarely in local dev, but every single time in CI.

@dimaqq
Copy link
Contributor

dimaqq commented Aug 24, 2021

.gitignore done in 91dbca8

@dimaqq dimaqq changed the title question: is there a reason why this repo doesn't have a .gitignore and poetry.lock question: is there a reason why this repo doesn't have poetry.lock checked in? Aug 24, 2021
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

No branches or pull requests

3 participants