search

expressjs / body-parser

Node.js body parsing middleware
MIT License
5.44k stars 727 forks source link

ci: fix errors in ci github action for node 8 and 9 #523

Closed inigomarquinez closed 5 months ago

inigomarquinez commented 6 months ago

This PR fixes nyc version to 14.1.1 when running tests in node 8 or node 9. nyc 15.xrequires a yargs package version that requires node >=10.

I've also added the latest versions of node (20, 21 and 22) to the matrix.

Related to https://github.com/jshttp/http-errors/issues/108

inigomarquinez commented 6 months ago

It seems that the tests fail for node >= 21, but not related to this PR itself, but to node itself.

wesleytodd commented 6 months ago

Ah, I am surprised we have more tests in here failing because of the query thing, but that is unrelated here. @jonchurch did you end up getting this fixed in that other branch? If so, mind dropping a link here so we don't have to repeat work to resolve it.

wesleytodd commented 6 months ago

I just tested locally and latest 22 does not fail. Do you want to update this to use 22.2.0? I am actually unsure what benefit we get from not just taking the latest in the major line, could we maybe just drop the minor specifier as well?

inigomarquinez commented 6 months ago

@wesleytodd , minor version removed from node 22. It still fails for node 21. Should I also removed minor version?

wesleytodd commented 6 months ago

Yeah I think we should remove the minor from all of them. We really want to be testing the latest in each major line without having to update all of them all the time.

inigomarquinez commented 5 months ago

@wesleytodd , I've removed the minor versions

@UlisesGascon , @carpasse , perhaps something to have in mind for all the other ci pipelines of the repositories we've been fixing/migrating 🤔

ctcpip commented 5 months ago

https://github.com/inigomarquinez/expressjs-body-parser/pull/1 addresses the failure in 21