Match upstream - Githubissues

mkopinsky / zxcvbn-php

Realistic PHP password strength estimate library based on Zxcvbn JS

MIT License

23 stars 9 forks source link

Match upstream #3

Closed clamburger closed 6 years ago

clamburger commented 6 years ago

With these last few changes we should now match upstream!

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 41

171 of 171 (100.0%) changed or added relevant lines in 14 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+11.3%) to 99.864%

Totals
Change from base Build 18:	11.3%
Covered Lines:	735
Relevant Lines:	736

💛 - Coveralls

mkopinsky commented 6 years ago

Thanks for all your work! I'd like to review this side-by-side with upstream before officially releasing.

One thing we should address (either by deleting or updating) is that the ZxcvbnTest is still skipped.

mkopinsky commented 6 years ago

re that broken Travis build - it's a one-line workaround to get around that failure in PHP 5.5, but I think we should just support 5.6+. 5.5 has been unsupported for almost 2 years.

clamburger commented 6 years ago

@mkopinsky I think this is done in terms of matching upstream and is ready to merge. I've tested against the top 1 million passwords from the rockyou password list and haven't found any further discrepancies.

mkopinsky commented 6 years ago

Can you add the scripts or documentation on how you did that comparison? I think that would be a useful tool for the future when/if upstream releases a new version.