llnode icon indicating copy to clipboard operation
llnode copied to clipboard

Upgrade GitHub Actions

Open cclauss opened this issue 3 years ago • 7 comments

  • https://github.com/actions/checkout/releases
  • https://github.com/actions/setup-node/releases
  • https://github.com/codecov/codecov-action/releases

cclauss avatar Aug 14 '22 12:08 cclauss

Codecov Report

Merging #403 (aa5ecf4) into main (ff75da7) will decrease coverage by 0.37%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
- Coverage   77.52%   77.14%   -0.38%     
==========================================
  Files          34       34              
  Lines        5027     5027              
==========================================
- Hits         3897     3878      -19     
- Misses       1130     1149      +19     
Impacted Files Coverage Δ
src/llscan.cc 60.56% <0.00%> (-1.86%) :arrow_down:
src/llv8.cc 71.67% <0.00%> (-0.27%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 14 '22 12:08 codecov-commenter

Hey @cclauss thanks for the PR but this work is already covered under these two PRs llvm - https://github.com/nodejs/llnode/pull/389 16/18 upgrade https://github.com/nodejs/llnode/issues/399 If you're trying for something else then please let us know or we can close this PR.

No9 avatar Aug 14 '22 16:08 No9

Let's merely upgrade the GitHub Actions.

cclauss avatar Aug 14 '22 17:08 cclauss

Excellent - The bump of actions, especially to the official setup-node, is very useful. As this could impact #389 from both a merge and feature perspective I'm going to land that first then merge this. Current plan is to do the merge of 389 by the end of this week - 2022-08-21.

Edit: I thought this fixed https://github.com/cclauss/llnode/blob/6ac110d4f6fea7b898b01e4b67669d502b15c07d/.github/workflows/push.yml#L35 But it looks like this PR needs to be updated https://github.com/actions/setup-node/pull/360 I still think it would be good to land this but the dep on my setup-node needs to be fixed.

No9 avatar Aug 15 '22 08:08 No9

Hey @cclauss #389 has merged - If you could rebase this I would be happy to help land

No9 avatar Aug 17 '22 22:08 No9

Rebased. Also fixed runs-on: to take a string, not an array because the runner is not self-hosted.

  • https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idruns-on

cclauss avatar Aug 18 '22 04:08 cclauss

Hey @cclauss There seems to be a tests that are on the edge of the timeouts for node14 with lldb12

Would you mind bumping https://github.com/nodejs/llnode/blob/ff75da7eb151cf8d7481eba9a1c6406a53c85ce5/test/plugin/inspect-test.js#L669 to 30000 and https://github.com/nodejs/llnode/blob/ff75da7eb151cf8d7481eba9a1c6406a53c85ce5/test/plugin/workqueue-test.js#L29 to 60000 And set the defaults to 40000 https://github.com/nodejs/llnode/blob/ff75da7eb151cf8d7481eba9a1c6406a53c85ce5/test/common.js#L45 https://github.com/nodejs/llnode/blob/ff75da7eb151cf8d7481eba9a1c6406a53c85ce5/test/common.js#L173

Thanks

No9 avatar Aug 18 '22 08:08 No9

@No9 Your re-review, please.

cclauss avatar Sep 13 '22 11:09 cclauss

Hey @cclauss I think these have landed in 404 as you requested https://github.com/nodejs/llnode/pull/404/files#diff-f3fc934cf0d89bdf07de358896ff90f1202585df812cf606206d1830a9949811R37

No9 avatar Sep 13 '22 22:09 No9

Closing in favor of #404

cclauss avatar Sep 14 '22 05:09 cclauss