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

Use sccache and vcpkg built-in GitHub Actions support in node-ci #2970

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tdcosta100
Copy link
Collaborator

After successful tests, I'm updating node-ci.yaml to use the enhancements of windows-ci.yaml to have better cache control and sharing with other workflows. Hopefully this will optimize the cache use and will speed-up the workflow in Windows.

@tdcosta100 tdcosta100 force-pushed the node-ci-sccache branch 9 times, most recently from a99dd7e to 86c6c79 Compare October 26, 2024 09:28
@tdcosta100 tdcosta100 marked this pull request as draft October 26, 2024 09:35
@tdcosta100 tdcosta100 force-pushed the node-ci-sccache branch 11 times, most recently from 81f412f to ff0d5d7 Compare October 26, 2024 15:37
@tdcosta100 tdcosta100 marked this pull request as ready for review October 26, 2024 16:01
@louwers
Copy link
Collaborator

louwers commented Oct 26, 2024

Shellcheck is reporting some issues:

.github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1009:info:6:3: The mentioned syntax error was in this simple command [shellcheck]
|
222 | run: |
| ^~~~
.github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1073:error:6:43: Couldn't parse this backtick expansion. Fix to allow more checks [shellcheck]
|
222 | run: |
| ^~~~
.github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1072:error:9:1: Fix any mentioned problems and try again [shellcheck]
|
222 | run: |
| ^~~~
.github/workflows/node-ci.yml:270:9: shellcheck reported issue in this script: SC1070:error:2:1: Parsing stopped here. Mismatched keywords or invalid parentheses? [shellcheck]
|
270 | run: |
| ^~~~
.github/workflows/node-ci.yml:270:9: shellcheck reported issue in this script: SC1133:error:2:1: Unexpected start of line. If breaking lines, |/||/&& should be at the end of the previous one [shellcheck]
|
270 | run: |
| ^~~~

You can install pre-commit locally with pre-commit install (after installing it with pip).

pre-commit run --all-files to run all checks

@tdcosta100 tdcosta100 force-pushed the node-ci-sccache branch 4 times, most recently from d909760 to 2b2a386 Compare October 27, 2024 04:29
@tdcosta100
Copy link
Collaborator Author

Shellcheck is reporting some issues:

.github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1009:info:6:3: The mentioned syntax error was in this simple command [shellcheck] | 222 | run: | | ^~~~ .github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1073:error:6:43: Couldn't parse this backtick expansion. Fix to allow more checks [shellcheck] | 222 | run: | | ^~~~ .github/workflows/node-ci.yml:222:9: shellcheck reported issue in this script: SC1072:error:9:1: Fix any mentioned problems and try again [shellcheck] | 222 | run: | | ^~~~ .github/workflows/node-ci.yml:270:9: shellcheck reported issue in this script: SC1070:error:2:1: Parsing stopped here. Mismatched keywords or invalid parentheses? [shellcheck] | 270 | run: | | ^~~~ .github/workflows/node-ci.yml:270:9: shellcheck reported issue in this script: SC1133:error:2:1: Unexpected start of line. If breaking lines, |/||/&& should be at the end of the previous one [shellcheck] | 270 | run: | | ^~~~

You can install pre-commit locally with pre-commit install (after installing it with pip).

pre-commit run --all-files to run all checks

Yeah, I discovered shellcheck does not support PowerShell scripts, so I converted everything to bash shell. That's sad, but okay, I will comply with it.

@tdcosta100 tdcosta100 force-pushed the node-ci-sccache branch 3 times, most recently from 77357f5 to 57d6b98 Compare October 27, 2024 05:34
@tdcosta100
Copy link
Collaborator Author

vcpkg doesn't work well with bash. I ended up updating the actionlint with the recommended way (it doesn't run shellcheck in PowerShell scripts), and it worked. I will submit a different PR, since it affects all workflow YAML files.

@tdcosta100 tdcosta100 force-pushed the node-ci-sccache branch 2 times, most recently from 9c82789 to 60239de Compare October 28, 2024 18:35
@tdcosta100
Copy link
Collaborator Author

vcpkg doesn't work well with bash. I ended up updating the actionlint with the recommended way (it doesn't run shellcheck in PowerShell scripts), and it worked. I will submit a different PR, since it affects all workflow YAML files.

Okay, resolved with a simple shell: pwsh. No more complaints about invalid shell code because it's not a shell code.

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