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

refactor(manager/poetry): use zod schema validation #23830

Merged
merged 5 commits into from
Aug 18, 2023
Merged

refactor(manager/poetry): use zod schema validation #23830

merged 5 commits into from
Aug 18, 2023

Conversation

zeshuaro
Copy link
Contributor

@zeshuaro zeshuaro commented Aug 11, 2023

Changes

Refactor to use zod schema validation for poetry manager

Context

  • Mainly, I want to work on the suggestion in this discussion poetry python version is not detected on dependency extraction #19144, which requires some parsing of the poetry/pyproject file:
  • I've raised other PRs recently and seems like the recommendation is to use zod for validating and parsing package/lock files, so I've done some refactoring first to make changes later easier

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

lib/modules/manager/poetry/__fixtures__/pyproject.2.toml Outdated Show resolved Hide resolved
lib/modules/manager/poetry/extract.ts Outdated Show resolved Hide resolved
lib/modules/manager/poetry/utils.ts Outdated Show resolved Hide resolved
@viceice viceice requested a review from zharinov August 15, 2023 19:43
@zeshuaro zeshuaro requested a review from viceice August 16, 2023 08:24
@zeshuaro zeshuaro requested a review from viceice August 17, 2023 13:15
viceice
viceice previously approved these changes Aug 17, 2023
@viceice viceice requested a review from secustor August 17, 2023 20:55
@zharinov
Copy link
Collaborator

Please update the branch, as we've merged Toml schema utils separately

@zharinov
Copy link
Collaborator

Please let me provide my own version of this refactoring. Code changes I want to ask for are quite big, so communication could take days. So please let me try to just refactor it on my own, hopefully you'll guys like it too.

@zeshuaro
Copy link
Contributor Author

Please let me provide my own version of this refactoring. Code changes I want to ask for are quite big, so communication could take days. So please let me try to just refactor it on my own, hopefully you'll guys like it too.

So do I still need to do anything for this PR? Or I’ll just close it and let you finish the refactoring?

@zharinov
Copy link
Collaborator

Please don't, just let me try to do the refactoring myself in another PR, and then you'll be able to work on your main goal

@zharinov
Copy link
Collaborator

Actually, let's make it another way. Let's merge your PR and I'll make one more refactoring on top of yours.

@zeshuaro
Copy link
Contributor Author

Actually, let's make it another way. Let's merge your PR and I'll make one more refactoring on top of yours.

Cool, I still want to work of the issue in this discussion #19144

I guess I’ll just wait for you to finish the changes first then?

@zharinov
Copy link
Collaborator

I guess I’ll just wait for you to finish the changes first then?

Yes, please

@rarkins rarkins added this pull request to the merge queue Aug 18, 2023
Merged via the queue into renovatebot:main with commit cec6fae Aug 18, 2023
36 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 36.52.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@zharinov
Copy link
Collaborator

Hi @zeshuaro, sorry for long pause. Please, take a look at the current zod-based implementation. It's not complete, but maybe 70–80% of what I wanted to achieve. Are you still interested in working on your original problem?

@zeshuaro
Copy link
Contributor Author

No problems, I actually already had a PR raised and merged which was then reverted due to some reported issues.

There is a new PR raised to address the problem: #24335

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants