Skip to content

Conversation

@qr243vbi
Copy link

@qr243vbi qr243vbi commented May 17, 2025

The main purpose of this commit is to fix empty provides_extra field, fix empty user and add ability to show metadata from local package

@qr243vbi qr243vbi force-pushed the master branch 5 times, most recently from 0abcb5b to c438323 Compare May 17, 2025 12:04
@danigm
Copy link
Contributor

danigm commented May 21, 2025

This PR is hard to review, there's just one commit with a lot of changes that moves code from __init__.py to utils.py. Please, try to split refactor changes and actual functionality changes or fixes so it's easier for anyone to review what changed.

@qr243vbi
Copy link
Author

This PR is hard to review, there's just one commit with a lot of changes that moves code from __init__.py to utils.py. Please, try to split refactor changes and actual functionality changes or fixes so it's easier for anyone to review what changed.

Writing code is much more difficult than reviewing it, so please look at the code, scrutinize it and ask your questions, I will respond

@danigm
Copy link
Contributor

danigm commented May 22, 2025

The description doesn't match with the actual diff in the PR, this PR does a lot of changes in a single commit that hides a big refactoring and does several unrelated changes at the same time so it's hard to tell if it's changing something relevant and can cause unexpected side effects.

In addition, the description mentions "fix metadata" and "fix bugs" but it's not clear to me which actual bugs is fixing, there's no link to existing issues or any example of the failure happening when using py2pack to fetch or generate a new spec file.

So I'm closing this PR. @qr243vbi if you want to get these changes in the py2pack code, please, try to follow these basic steps:

  • Write a good description in the PR, explaining what fixes and why. Provide examples if you have any and how to reproduce.
  • Make each commit both small and atomic.
  • Explain the “what” and “why” of your change in the commit message.

Here you have some references about how to create a better PR/commit:

@danigm danigm closed this May 22, 2025
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