I could see an argument made for making it a constant in the _test_
(EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the
number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <kas...@gmail.com>
wrote:

> I’m -1 on changing that. I think the constant 72 is used directly such
> that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.
>
> With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some
> other contributor wouldn’t make any tests fail.
>
> Since that constant isn’t meant to change without us thinking more about
> it, I’d say we keep the code as is.
>
> Thanks for starting to contribute!
>
> Den 9. jan. 2018 kl. 09.56 skrev artur.belja...@internet.ee:
>
> Magic number 72 is used many times throughout the tests, which is actually
> present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED
> constant. I propose to replace that magic number either by the constant or
> method call.
>
> I have never contributed to such large open source project as Rails.
> Please let me know your thoughts.
>
>
>
> https://github.com/rails/rails/blob/master/activemodel/
> test/cases/secure_password_test.rb
> https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed
> 2ec915bd9b/activemodel/lib/active_model/secure_password.rb
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> Kasper
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Reply via email to