Skip to content
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

Update deps, require node 8 #105

Merged
merged 3 commits into from
Nov 12, 2019
Merged

Update deps, require node 8 #105

merged 3 commits into from
Nov 12, 2019

Conversation

bendrucker
Copy link
Collaborator

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
Copy link
Contributor

charmander commented Oct 2, 2019

Breaking changes also worth considering for a major release:

@bendrucker
Copy link
Collaborator Author

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
Copy link
Owner

brianc commented Oct 14, 2019

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
Copy link
Collaborator Author

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
Copy link

codecov-io commented Nov 12, 2019

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%) ⬇️

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 bendrucker merged commit 09ea625 into master Nov 12, 2019
@bendrucker bendrucker deleted the update-deps-node-8 branch November 12, 2019 21:38
@bendrucker
Copy link
Collaborator Author

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
Copy link

gajus commented Jun 8, 2020

What is the correct way to parse the array now?

This broke gajus/slonik#182

@gajus
Copy link

gajus commented Jun 8, 2020

This is how I fixed it gajus/slonik@042669d, but not sure if that was the expected way.

@charmander
Copy link
Contributor

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

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.

Error with Buffer() Deprecated
5 participants