Artur, send a pull request :) -- Kasper
On 21 Feb 2018, 01.14 +0100, Artur Beljajev <artur.belja...@internet.ee>, wrote: > Should I create a ticket on Github first? > > On Saturday, January 20, 2018 at 3:28:49 PM UTC+2, Kasper Timm Hansen wrote: > > Yep, that sounds good too, Kerri. > > > > Artur, go right ahead! > > > > -- > > Kasper > > > > On 10 Jan 2018, 15.37 +0100, Kerri Miller <kerr...@gmail.com>, wrote: > > > 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.b...@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/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/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-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. > -- > 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.
