-
Notifications
You must be signed in to change notification settings - Fork 283
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
build: upgrade & consolidate @types/node at v18.11.9 - NodeJS typings #3067
build: upgrade & consolidate @types/node at v18.11.9 - NodeJS typings #3067
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but:
- codegen is failing (I'm not familiar with this error, maybe it's just a flake)
- some fabric tests are failing on go deployment, could you investigate if that's not caused by: https://github.com/hyperledger/cacti/commit/21fd747e37d20e2427ad6e86b6d5d15435fbc660 ?
d3054d1
to
e2931b8
Compare
@outSH The I have a possible fix for this but it requires refactoring all the codegen scripts in all the packages so it'll take some time, in the meantime most of the time if you retry the codegen script (it can be re-tried individually so you don't need to re-trigger the entire ci.yaml workflow) then it usually works right away. For the fabric tests: My guess is that they went broke even before [21fd747] because the older AIO fabric images are all flaky now (they tend go bad over time because of auto-upgrades that we can't control), BUT that's just strictly a guess for now and I'm looking into it. |
@outSH I've submitted a separate PR that should fix or at least drastically reduce the flakiness of the Fabric tests: https://github.com/hyperledger/cacti/pull/3082 My investigation (so far) has concluded that there's no link between the Fabric tests aging out and becoming flaky and this current PR, but of course you never know... :-) |
1. The default/suggested NodeJS version we use and recommend is v18 LTS but there were a lot of places in the code that still declared v14 and v16 both of which are now EOL. 2. This was/is working fine but eventually we'll hit some compiler issue where the upgrade will be forced so it's most likely wiser to do this little quality of life/housekeeping change now when it's not urgent. Signed-off-by: Peter Somogyvari <[email protected]>
e2931b8
to
55760f9
Compare
but there were a lot of places in the code that still declared v14 and
v16 both of which are now EOL.
where the upgrade will be forced so it's most likely wiser to do this little
quality of life/housekeeping change now when it's not urgent.
Signed-off-by: Peter Somogyvari [email protected]
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.