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

fix: enable logging for npm install action #1559

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Conversation

weboko
Copy link
Collaborator

@weboko weboko commented Sep 11, 2023

Problem

Due to broken behavior of npm action release PR was broken and wasn't able to succeed due to npm ci not working on release. It is expected because of package-lock and package filed being out of sync so npm i should be used. For some reason action was misfunctioning and using npm ci.

My guess right now is that for some reason github.ref context is not in a form of branch (like if was before) but in a form of refs/pull/1559/merge the action wrongly uses npm ci on release PR, which it shouldn't.

Solution

Make use of a following pseudo-code:

...
if: IS_MASTER || IS_RELEASE
run: npm i
...
if: IS_PR && IS_NOT_RELEASE
run: npm ci

For now just enabling debugging in CI to observe.

And yes, it would be good to keep using npm ci to reduce downloading packages each time.

Notes

@github-actions
Copy link

github-actions bot commented Sep 11, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 28.79 KB (0%) 576 ms (0%) 479 ms (+43.31% 🔺) 1.1 s
Waku Simple Light Node 309.78 KB (0%) 6.2 s (0%) 1.1 s (-3.95% 🔽) 7.3 s
ECIES encryption 28.68 KB (0%) 574 ms (0%) 434 ms (+47.04% 🔺) 1.1 s
Symmetric encryption 28.68 KB (0%) 574 ms (0%) 393 ms (-1.17% 🔽) 966 ms
DNS discovery 118.59 KB (0%) 2.4 s (0%) 758 ms (+0.28% 🔺) 3.2 s
Privacy preserving protocols 122.6 KB (0%) 2.5 s (0%) 646 ms (+4.25% 🔺) 3.1 s
Light protocols 29.22 KB (0%) 585 ms (0%) 384 ms (+18.45% 🔺) 969 ms
History retrieval protocols 28.34 KB (0%) 567 ms (0%) 354 ms (+4.28% 🔺) 921 ms
Deterministic Message Hashing 5.64 KB (0%) 113 ms (0%) 137 ms (+23.56% 🔺) 249 ms

@weboko weboko changed the title fix: enable npm ci to targeted branches fix: enable logging for npm install action Sep 11, 2023
@weboko weboko marked this pull request as ready for review September 11, 2023 13:29
@weboko weboko requested a review from a team as a code owner September 11, 2023 13:29
@weboko weboko merged commit 833b02a into master Sep 11, 2023
11 of 12 checks passed
@weboko weboko deleted the weboko/fix-ci-release branch September 11, 2023 13:30
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.

1 participant