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

Migrate to pyproject.toml #71

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

deronnax
Copy link
Contributor

pyproject.toml is the way to go

pyproject.toml Outdated Show resolved Hide resolved
@deronnax deronnax requested a review from oz123 September 21, 2023 11:50
@deronnax
Copy link
Contributor Author

@oz123 is it good for you now?

@oz123
Copy link
Contributor

oz123 commented Sep 29, 2023

I'm not going to be able to merge this PR.. Not an owner here. I'm just sharing my thoughts.
You should also remove the optional dependancy on pipenv.

@deronnax
Copy link
Contributor Author

deronnax commented Oct 5, 2023

@oz123 I want to do 1:1, removing the pipenv dependencies, even undue, would be more than the minimum. I will do if the project owner ask for it, though. If he ever shows up.

@deronnax
Copy link
Contributor Author

deronnax commented Oct 7, 2023

@yeisonvargasf can I have a workflow run please?

@deronnax
Copy link
Contributor Author

deronnax commented Oct 9, 2023

@yeisonvargasf it's ready to be merged if you're OK with that. Btw, I think we should follow @oz123 's recommandation: the optional requirements on an old version of Pipenv doesn't make sense (it can't be enforced that way).

@yeisonvargasf
Copy link
Member

Hi @oz123, thank you for reviewing this PR; I appreciate your help here.

@deronnax, thank you for this PR; I agree with the @oz123 recommendation about removing the Pipenv constraint; My idea is to remove it after updating the PipfileUpdater. Although the pipenv constraint can't be enforced that way (If already installed), I think that works in the case of a fresh CI environment with no pipenv pre-installed.

Please let me know if the comment about the CI case is wrong; if the constraint doesn't work in that case, we can remove it in this same PR; otherwise, I'm happy to merge this as it is.

@yeisonvargasf yeisonvargasf requested review from yeisonvargasf and removed request for oz123 October 10, 2023 21:39
@yeisonvargasf yeisonvargasf merged commit 3323cff into pyupio:master Oct 11, 2023
1 check passed
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.

3 participants