Skip to content

Conversation

@paragonie-scott
Copy link
Collaborator

#154 redux

README.md Outdated

### Direct Installation (Phar)

Download the PHP Archive and public key. Extract
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this sentence was supposed to be finished but wasn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. @_@

@defuse
Copy link
Owner

defuse commented Apr 3, 2016

This is really great work!

@paragonie-scott
Copy link
Collaborator Author

Thanks. Fixed the snafu. :)

```php
try {
$ciphertext = \Defuse\Crypto\Crypto::encrypt("Test message", $key);
} catch (\Defuse\Crypto\Exception\CryptoTestFailedException $ex) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could throw a bunch of different types of exceptions, like CannotPerformOperationException. I have a new philosophy when it comes to exceptions: Don't catch them unless you can actually do something with the result (if nothing can do something with the result, then the process terminates and that's okay).

I think in our example code we shouldn't put try-catch but instead a comment mentioning it can throw various things (with the exception of InvalidCiphertextException).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Don't enable "copy-paste programmers" to have a silent failure they didn't consciously write themselves.

@defuse
Copy link
Owner

defuse commented Apr 5, 2016

I'm pulling this into a branch to work on documentation.

@defuse
Copy link
Owner

defuse commented Apr 5, 2016

Oh, it's already on a branch. I'll just commit there :)

@paragonie-scott
Copy link
Collaborator Author

👍

@paragonie-scott
Copy link
Collaborator Author

Is this superceded by #226?

@defuse
Copy link
Owner

defuse commented Apr 11, 2016

Not yet, I'm probably still going to use a lot of it.

@defuse
Copy link
Owner

defuse commented Apr 24, 2016

I looked at this a lot while doing #226. I think that PR includes (at least a rewrite of) everything that was in this one.

@defuse defuse closed this Apr 24, 2016
@paragonie-scott paragonie-scott deleted the v2.0-readme-2 branch April 24, 2016 20:19
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.

3 participants