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

Enable include-hidden-files for actions/upload-artifact@v4 #245

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Sep 10, 2024

Looks like we now need to set this option when uploading coverage, since the default file starts with a leading . and is likely considered as ignored.

https://github.com/actions/upload-artifact/releases/tag/v4.4.0

image

This would explain why the coverage check started failing on notebook: jupyter/notebook#7456

@jtpio jtpio changed the title Enable include-hidden-files to actions/upload-artifact@v4 Enable include-hidden-files for actions/upload-artifact@v4 Sep 10, 2024
@jtpio jtpio added the bug Something isn't working label Sep 10, 2024
Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you @jtpio!

@krassowski
Copy link
Member

I think it is safe to merge as the PR fixes exactly the test that is failing.

@jtpio
Copy link
Member Author

jtpio commented Sep 10, 2024

Yes, also the motivation for the original upstream change was mostly for security reasons: actions/upload-artifact#598

But in our case we select only one file to be upload (the coverage file).

@jtpio
Copy link
Member Author

jtpio commented Sep 10, 2024

For reference it also seems to be affecting other repos using this action, for example ipykernel: https://github.com/ipython/ipykernel/actions/runs/10793224692/job/29935156641

@krassowski krassowski merged commit de8364a into jupyterlab:main Sep 10, 2024
24 of 28 checks passed
@jtpio
Copy link
Member Author

jtpio commented Sep 10, 2024

This seems to be fixing the upload part.

Although the report-coverage actions now seems to be failing with:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants