Skip to content

Conversation

@paragonie-scott
Copy link
Collaborator

Reference: #125

@paragonie-scott
Copy link
Collaborator Author

@defuse
Copy link
Owner

defuse commented Dec 11, 2015

Fuck, I want to reply to one of @paragonie-scott's comments, but I can't remember which commit it's in, and GitHub doesn't give me the link.

No, it doesn't return false. You threw a return; in encryptFile() and the unit tests didn't catch it.

So I accidentally added a return statement somewhere in the function, and that broke it, without breaking the unit tests, and returning true ensures that they'll catch it in the future? Could you document that (or just point me to a commit), I'd like to improve the unit tests so that they'll catch it.

@defuse
Copy link
Owner

defuse commented Dec 11, 2015

@paragonie-scott: I think you addressed all my comments, is this ready to merge?

@paragonie-scott
Copy link
Collaborator Author

I believe it's ready.
On Dec 11, 2015 2:03 AM, "Taylor Hornby" notifications@github.com wrote:

@paragonie-scott https://github.com/paragonie-scott: I think you
addressed all my comments, is this ready to merge?


Reply to this email directly or view it on GitHub
#129 (comment)
.

defuse added a commit that referenced this pull request Dec 11, 2015
Prevent partial reads/writes, and a bunch of other stuff.
@defuse defuse merged commit d871915 into defuse:v2.0 Dec 11, 2015
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.

2 participants