brianc / node-pg-types

Type parsing for node-postgres
268 stars 55 forks source link

Update deps, require node 8 #105

Closed bendrucker closed 4 years ago

bendrucker commented 4 years ago

This would require a major version bump, which I think we should be doing anyway as numeric/binary oriented changes have been piling up in master. @brianc are you ok with going ahead with a major release of node-postgres as well? Node 4's EOL was about 1.5 years ago. 8's is coming up (December).

Closes #104

charmander commented 4 years ago

Breaking changes also worth considering for a major release:

bendrucker commented 4 years ago

I think we should just get a major release out and evaluate dates and big ints for a later major. We haven't been doing many of these releases. Removing lib/arrayParser.js seems easy/uncontroversial.

@brianc Thoughts on doing a major release for Node 8 and then doing a future major for Node 12 that incorporates BigInt + Date changes?

brianc commented 4 years ago

Thoughts on doing a major release for Node 8 and then doing a future major for Node 12 that incorporates BigInt + Date changes?

Yeah I'm down with that. You want a major bump in node-posgres core as well right? I can sweep in a couple other breaking changes we wanted to make too.

bendrucker commented 4 years ago

You want a major bump in node-posgres core as well right?

Yup, major bump here, and then a major bump in node-postgres updating the dep.

codecov-io commented 4 years ago

Codecov Report

Merging #105 into master will increase coverage by 0.6%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #105     +/-   ##
=========================================
+ Coverage   88.01%   88.62%   +0.6%     
=========================================
  Files           5        4      -1     
  Lines         217      211      -6     
=========================================
- Hits          191      187      -4     
+ Misses         26       24      -2
Impacted Files Coverage Δ
index.js 75% <ø> (-2.28%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 96d238f...0d2ea80. Read the comment docs.

bendrucker commented 4 years ago

Finally took the time to merge this and release 3.0 to npm. Happy to take on further breaking changes in 4.0 which might be node 12+ assuming it comes early 2020.

@brianc Do you need any help landing other changes to node-postgres? If not I can just open the PR bumping the pg-types dep and setting engines.node.

gajus commented 4 years ago

What is the correct way to parse the array now?

This broke https://github.com/gajus/slonik/issues/182

gajus commented 4 years ago

This is how I fixed it https://github.com/gajus/slonik/commit/042669dab03421e18e8dda270c92a4089cbdedd7, but not sure if that was the expected way.

charmander commented 4 years ago

@gajus That works, yes. You can also write parseArray(arrayValue, typeParser.parse).