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

Add ircmaxell/password-compat as a dependency #10

Closed
dosten opened this issue Mar 25, 2015 · 4 comments · Fixed by #11
Closed

Add ircmaxell/password-compat as a dependency #10

dosten opened this issue Mar 25, 2015 · 4 comments · Fixed by #11
Labels

Comments

@dosten
Copy link
Contributor

dosten commented Mar 25, 2015

I'm running the demo in 5.4 and when i go to the login page i get an error because the password_hash function doesn't exists in 5.4, we can add ircmaxell/password-compat as a dependency to solve this. if you are ok with this i can send a PR :)

PD: Sorry for my english, i'm noob

@javiereguiluz
Copy link
Member

Thanks for reporting this problem. I consider it a bug because we intend to support even PHP 5.3, so this problem shouldn't happen. And given that the ircmaxell/password-compat library doesn't affect the application if you use PHP 5.5 or higher, I think it's safe to always include it.

We'll be very happy if you submit a PR with this change. Thanks!

PS: I know that it may look weird, but in our composer.json file we sort dependencies alphabetically and align them with white spaces. Please keep in mind this weirdness when adding the new dependency. Thanks!

@stof
Copy link
Member

stof commented Mar 25, 2015

looks like a good idea to me. Our demo should run on as many versions of PHP as possible to make it simpler to use.

javiereguiluz added a commit that referenced this issue Mar 25, 2015
This PR was merged into the master branch.

Discussion
----------

Added ircmaxell/password-compat as a dependency

This PR fixes #10

Commits
-------

82925b1 Added ircmaxell/password-compat as a dependency
@stof
Copy link
Member

stof commented Mar 25, 2015

@javiereguiluz I would be in favor of removing the alignment for the same reason we don't align => in arrays (even though incenteev/composer-parameter-handler already puts the min length high enough for most packages...)

@dosten
Copy link
Contributor Author

dosten commented Mar 25, 2015

@stof 👍

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

Successfully merging a pull request may close this issue.

3 participants