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

[5.5] Add a str_hash() helper #20169

Closed
wants to merge 1 commit into from
Closed

[5.5] Add a str_hash() helper #20169

wants to merge 1 commit into from

Conversation

m1guelpf
Copy link
Contributor

@m1guelpf m1guelpf commented Jul 19, 2017

This PR adds the new helper that will replace bcrypt() in the future, as requested by Taylor in my last PR.

@taylorotwell Should I also deprecate (adding a @deprecated docblock & triggering a deprecated error) bcrypt() or are we leaving this for another Laravel release?

@tillkruss tillkruss changed the title Add a str_hash() helper [5.5] Add a str_hash() helper Jul 19, 2017
@taylorotwell
Copy link
Member

Typically we would include the method on the Str class as well.

@m1guelpf
Copy link
Contributor Author

m1guelpf commented Jul 19, 2017

@taylorotwell Should I move the app() method to the Str class or use the Hash facade?

@devcircus
Copy link
Contributor

Really seems out-of-place in string helpers. Maybe the "str_" prefix isn't the best naming choice for bcrypt replacement.

@lucasmichot
Copy link
Contributor

Yeah @taylorotwell I agree witj @devcircus
The str_ really does not match

@taylorotwell
Copy link
Member

I'm not sure this is even needed to be honest. Just use Hash::make().

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.

4 participants