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

[WIP] v2.0.0 (only ciphers, not hashes) #3

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

fanatid
Copy link

@fanatid fanatid commented Apr 17, 2016

@calvinmetcalf can you check code before tests?
It is not simply removing because I want strict following by node crypto. Is CipherBase/CipherivBase/.. good decision?

@fanatid fanatid changed the title v2.0.0 (only ciphers, not hashes) [WIP] v2.0.0 (only ciphers, not hashes) Apr 17, 2016
@fanatid
Copy link
Author

fanatid commented Apr 29, 2016

default encoding should be utf8
PR in hash-base: browserify/hash-base#4

@calvinmetcalf
Copy link
Contributor

in node the cipher and cipheriv are based on each other so there isn't really much need for a separate.

Also some of these changes are things that probably seem clearer to you, but seam less clear to me and are probably overall exactly the same clarity wise. These add some needless churn to the diff so could you separate out the more stylistic changes from the functional ones so I can review them separately?

We should also probably update the default encoding in a sperate pull because that is probably what's hosing the tests.

(was on vacation last week, then got really sick wtihin hours of getting home this week so am still digging out)

@fanatid
Copy link
Author

fanatid commented Nov 2, 2016

Updated.
Will actual when Node v0.12 will not be supported and when nodejs/node#9405 / nodejs/node#9398 will be merged.

@dcousens
Copy link
Member

dcousens commented May 24, 2017

@fanatid can you action this?
Potentially merge #8 prior

@fanatid
Copy link
Author

fanatid commented May 24, 2017

@dcousens if I remember correctly this WIP because we decide wait until 0.12 will out of support? you free to change or create new PR based on this

@dcousens
Copy link
Member

OK, 0.12 has been dropped, 4 is the LTS now... so we can merge?

@fanatid
Copy link
Author

fanatid commented May 24, 2017

I think yes, but safe-buffer should be added
and final word belongs to @calvinmetcalf

@ljharb ljharb marked this pull request as draft November 16, 2024 06:44
@ljharb
Copy link
Member

ljharb commented Nov 16, 2024

@fanatid if this is still relevant, please rebase it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants