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

Refactor password #91

Merged
merged 10 commits into from
Aug 8, 2015
Merged

Refactor password #91

merged 10 commits into from
Aug 8, 2015

Conversation

ximex
Copy link
Member

@ximex ximex commented Jul 12, 2015

please review this code and give feedback

@ximex ximex self-assigned this Jul 12, 2015
@ximex ximex added this to the v3.1 milestone Jul 12, 2015
@ximex ximex mentioned this pull request Jul 12, 2015
@Fasse Fasse removed the change label Jul 31, 2015
@Fasse
Copy link
Member

Fasse commented Jul 31, 2015

Why do you create a new issue? This is also 89, an improved new hashing method :)

I got the following error when using the branch:

Parse error: syntax error, unexpected T_OBJECT_OPERATOR in /Users/Jennifer/Sites/GitHub/admidio/adm_program/system/classes/autologin.php on line 119

We should not code so compact as in line 119. This is not easy to read and complicated to debug. Please write more lines and dont combine multiple method calls in one line.

@ximex
Copy link
Member Author

ximex commented Jul 31, 2015

What do you mean with "Why do you create a new issue?" This isn't an issue. This is a pull request.
This is what i said in the mails. we could also use pull requests and not only issues ;-)

I think i found the problem: https://stackoverflow.com/questions/13388541/php-parse-error-syntax-error-unexpected-t-object-operator/13388570#13388570

@ximex
Copy link
Member Author

ximex commented Jul 31, 2015

b68e7df should fix this

@Fasse
Copy link
Member

Fasse commented Aug 8, 2015

Why are here conflicts and how can I solve them?

I dont know how I can see these conflicts. I can to a branch merge to the master but then the Pull-Request is still open ...

@ximex
Copy link
Member Author

ximex commented Aug 8, 2015

remove merge from master
rebased on master
force update branch

ximex pushed a commit that referenced this pull request Aug 8, 2015
@ximex ximex merged commit e86fe63 into master Aug 8, 2015
@ximex ximex deleted the refactor-password branch August 8, 2015 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants