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

Add source distribution upload to the PyPi-cd workflow #3213

Merged
merged 9 commits into from
Feb 23, 2025

Conversation

liorsve
Copy link
Contributor

@liorsve liorsve commented Feb 19, 2025

Issue link

This Pull Request is linked to issue (): [#3189 ]

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

@liorsve liorsve requested a review from a team as a code owner February 19, 2025 21:24
@Yury-Fridlyand Yury-Fridlyand added the CI/CD CI/CD related label Feb 19, 2025
@liorsve
Copy link
Contributor Author

liorsve commented Feb 19, 2025

@@ -198,31 +198,64 @@ jobs:
path: python/wheels
if-no-files-found: error

publish-to-pypi:
build-source-dist-and-publish-to-pypi:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to create another job to build sources and add it there as a dependency (needs section)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will require to upload the build files and then download them, I think it's unnecessary and makes sense to keep it in the same job due to this reason

Copy link
Collaborator

@barshaul barshaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


LGTM - only comment regarding the .RUNNER change - please address the comment before merging

Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
Signed-off-by: lior sventitzky <[email protected]>
@liorsve liorsve force-pushed the lior/publish-dist-pypi branch from ccdc283 to 3f7cf8a Compare February 23, 2025 11:44
@liorsve liorsve merged commit 3789f14 into main Feb 23, 2025
17 checks passed
@liorsve liorsve deleted the lior/publish-dist-pypi branch February 23, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD CI/CD related python Python wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants