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

diff-npm-package: create reports folder #3623

Merged
merged 2 commits into from Jun 1, 2022
Merged

Conversation

yaacovCR
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented May 31, 2022

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 97f032c
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/62975d48be734a00086161d5
😎 Deploy Preview https://deploy-preview-3623--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@yaacovCR yaacovCR requested a review from a team May 31, 2022 12:47
resources/diff-npm-package.ts Outdated Show resolved Hide resolved
@saihaj saihaj requested a review from IvanGoncharov June 1, 2022 12:40
@IvanGoncharov IvanGoncharov merged commit 7532a5c into graphql:main Jun 1, 2022
@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 1, 2022

this is still actually (sadly) not working, although CI now is.

https://github.com/graphql/graphql-js/pull/3623/checks#step:7:7

Run actions/upload-artifact@v3
with:
name: npm-dist-diff.html
path: ./reports/npm-dist-diff.html
if-no-files-found: ignore
No files were found with the provided path: ./reports/npm-dist-diff.html. No artifacts will be uploaded.

will try to submit a fix

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Jun 1, 2022

never mind, sorry for the error, script doesn't generate a report if no diff is found.

on the other hand, the code change is a bit odd, as we are testing to see whether the file is present, and then if not, creating the directory with the path to the file. i guess it is working, but still a bit off.

yaacovCR added a commit to yaacovCR/graphql-executor that referenced this pull request Jun 1, 2022
graphql#3623 creates the reports folder via the report path, rather than the reports folder path. This did not trigger an error, because no changes were found.
IvanGoncharov pushed a commit that referenced this pull request Jun 2, 2022
#3623 creates the reports folder via the report path, rather than the reports folder path. This did not trigger an error, because no changes were found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants