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

ci: fix failing tests after update to upload-artifact v4 #273

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

chrispap95
Copy link
Collaborator

upload-artifact v4 cannot overwrite artifacts and this causes the tests to fail.

@chrispap95
Copy link
Collaborator Author

So it fixes the overwrite issue in the CI wheel tests. The real question is if it handles correctly the full wheel building process. Perhaps someone more knowledgeable should take a look at these changes.

@lgray
Copy link
Collaborator

lgray commented Feb 15, 2024

I personally don't use this action so I'm not sure if this will be correct.

Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

Sorry, I don't have time to give a full useful review right this minute, but I can at least point you to scientific-python/cookie#350 which should show how to simplify things a bit.

Just make sure that both the upload and download are both on v4 or you'll have problems.

@chrispap95
Copy link
Collaborator Author

chrispap95 commented Feb 16, 2024

I implemented the changes as they are suggested in scientific-python/cookie#350
I am quite confident they are okay. I will merge tonight if there are no objections.

Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

LGTM. 👍 Things are passing, so I'd say hit it! 🚀

@lgray lgray merged commit 511d546 into main Feb 17, 2024
12 checks passed
@lgray lgray deleted the fix-upload-artifact-issue branch February 17, 2024 01:00
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