Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.
// Kerri On Wed, Jan 10, 2018 at 3:19 PM, <artur.belja...@internet.ee> wrote: > I would be happy to do this! > > > On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote: >> >> Yep, we could do that 👍 >> >> Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <kerr...@gmail.com>: >> >> 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.b...@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/ff657e73f0b61a7e224a9f15 >>> 8467ed2ec915bd9b/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-co...@googlegroups.com. >>> To post to this group, send email to rubyonra...@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-co...@googlegroups.com. >>> To post to this group, send email to rubyonra...@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-co...@googlegroups.com. >> To post to this group, send email to rubyonra...@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.
