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

Retire the use of poetry-semver #43

Closed
valeriupredoi opened this issue Apr 20, 2023 · 6 comments
Closed

Retire the use of poetry-semver #43

valeriupredoi opened this issue Apr 20, 2023 · 6 comments

Comments

@valeriupredoi
Copy link
Contributor

Hey guys, I am in the process of updating support for python311 for our package and since we use prospector, and prospector depends on requirements-detector, I went about and started updating the conda packages for both prospector and your package; the one problem that I am facing while updating the conda package for requirements-detector is that you depend on, and use poetry-semver which doesn't have a conda package candidate, nor does it have a feedstock, nor is it developed any more, it looks like it's now archived. Would you mind if I opened a PR to replace it and its functionality with semver instead? Many thanks in advance, cheers 🍺

@carlio
Copy link
Member

carlio commented Apr 20, 2023

Hi there,

I have looked into this in the past - #40 - and decided not to do this.

The simple fact is that by adding poetry to requirements-detector , and therefore prospector , this would add a huge amount of additional dependencies to CI processes that are using prospector. Poetry has many dependencies.

poetry-semver is deprecated but does currently do everything that it needs to do, and my attempts at writing its parsing into requirements-detector proved more complex than I liked.

I will not merge #44 for this reason as it has too many consequences for people's build pipelines as it would introduce additional dependency constraints onto prospector, most of which are transient from poetry and are not needed. However this would likely lead to conflicts with version constraints of the projects that use prospector and I don't like getting lots of bug reports by breaking builds.

@carlio carlio closed this as completed Apr 20, 2023
@carlio
Copy link
Member

carlio commented Apr 20, 2023

Note that an option that I would accept is simply to copy in the semver package from poetry-semver and bundle it. This way it does not have problems with deprecated (or missing) packages and it does not drag in all of poetry.

Doing this basically just freezes the version parsing at the exact same state that poetry-semver is in but this is exactly what's happening by pinning an old, unmaintained separate package.

If you want to do this, go ahead - if not I will get around to it tomorrow.

@valeriupredoi
Copy link
Contributor Author

Hi @carlio many thanks for getting back, mate! I understand the idea behind the deps brought in by poetry but I don't think it's as bad as you think, at least on conda-forge:

  • poetry on its own needs 95 packages
  • poetry + prospector need 126 packages
  • prospector on its own needs 63 packages

So at least statistically, numbers are not that bad at all. Do you still not want to have poetry in as a dep? If still no, then, the idea of getting the poetry-semver source code in is a good idea, I'll have a stab at it 🍺

@carlio
Copy link
Member

carlio commented Apr 20, 2023

Well, 126 is twice the amount of 63, which seems to me like something which will double the chance of prospector causing a version constraint problem for people using it.

So yes I definitely want to avoid doubling the dependencies involved!

@valeriupredoi
Copy link
Contributor Author

sounds like a plan! Then if that's OK with you I'll try plop the poetry-semver code in requirements-detector 👍 🍺

@carlio
Copy link
Member

carlio commented Apr 20, 2023

Yep sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants