Skip to content

Conversation

@lstocchi
Copy link
Contributor

No description provided.

@lstocchi lstocchi force-pushed the migrateNode20 branch 8 times, most recently from 1e59b91 to 008a13c Compare February 20, 2025 17:49
uses: actions/setup-node@v3
with:
node-version: '10.x'
node-version: '20.x'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse my ignorance, I'm not really familiar with this repo and Github workflows.
But why the version is hard coded in here instead of "node-version: ${{ matrix.node }}" like on line 28?
If my question doesn't make sense, please feel free to ignore me :)

Copy link
Contributor Author

@lstocchi lstocchi Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no strong opinion to why a hard coded version is better than a matrix in this case and it could be changed. Obviously, in general cases, running many matrix tests is more expensive. In this case, the tests are minimal .... it just runs matrix on linux to identify some potential errors with a specific version of node (to see if some func has been deprecated/removed and the code break), and on win/Mac just targets the version which should be used at runtime.
I'm not working with azure devops anymore (I left this field 3+ years ago) so I'm not really updated about it but it seems they recommend node 20. This is why I changed it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah makes sense, much appreciated!

@KChavez04
Copy link

This PR has been open for a bit. Is there is anything that others can do to push it along? I am interested helping move it forward, if possible. Thank you!

@lstocchi
Copy link
Contributor Author

lstocchi commented May 22, 2025

@KChavez04 unfortunately i'm not responsible anymore for this project and i don't have bandwidth to work on it.
I still have to verify if this PR works, so if you have a chance to test it and confirm it works it would be great.

BTW i'll try to find 1 hour on monday to complete it as it is very close to be released. Sorry for the trouble

@lstocchi lstocchi force-pushed the migrateNode20 branch 4 times, most recently from 0a6389f to 2b55cf8 Compare May 27, 2025 15:27
@lstocchi
Copy link
Contributor Author

Just tested and it does not work bc of an error with the node-fetch lib.
I'm closing this PR as i don't have time to work on it. I hope someone else will jump in to maintain this extension soon.

@lstocchi lstocchi closed this May 27, 2025
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.

3 participants