[webkit-reviews] review denied: [Bug 198391] Add support of zxcvbn password strength checker to bugs.webkit.org website. : [Attachment 370994] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has denied lingho at apple.com
<lingho at apple.com>'s request for review:
Bug 198391: Add support of zxcvbn password strength checker to bugs.webkit.org
website.
https://bugs.webkit.org/show_bug.cgi?id=198391

Attachment 370994: Patch

https://bugs.webkit.org/attachment.cgi?id=370994&action=review




--- Comment #3 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 370994
  --> https://bugs.webkit.org/attachment.cgi?id=370994
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370994&action=review

r- only to update Bugzilla/Install/Requirements.pm to add the
Data::Password::zxcvbn module as required.  The rest of the changes are
optional.

> Websites/bugs.webkit.org/Bugzilla/Config/Auth.pm:115
> +		   'letters_numbers_specialchars', 'zxcvbn' ],

Nit:  I hadn't heard of zxcvbn before, so using something a little more
description (like 'zxcvbn_password_checker') would be easier to grok the first
time you see it.

Not necessary to land the patch.

> Websites/bugs.webkit.org/Bugzilla/User.pm:34
> +use Data::Password::zxcvbn 'password_strength'; # WEBKIT_CHANGES

I think it's more idiomatic Perl to use qw() here:

use Data::Password::zxcvbn qw(password_strength); # WEBKIT_CHANGES

--

You also need to update Bugzilla/Install/Requirements.pm to list the
Data::Password::zxcvbn module as required.

> Websites/bugs.webkit.org/Bugzilla/User.pm:2492
> +    } elsif ($complexity_level eq 'zxcvbn') {

Update 'zxcvbn' if you changed the name above.