Yep, we could do that 👍 > Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <kerri...@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 > <mailto: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 >> <mailto: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/master/activemodel/test/cases/secure_password_test.rb> >> https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.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-core+unsubscr...@googlegroups.com >> <mailto:rubyonrails-core+unsubscr...@googlegroups.com>. >> To post to this group, send email to rubyonrails-core@googlegroups.com >> <mailto:rubyonrails-core@googlegroups.com>. >> Visit this group at https://groups.google.com/group/rubyonrails-core >> <https://groups.google.com/group/rubyonrails-core>. >> For more options, visit https://groups.google.com/d/optout >> <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 > <mailto:rubyonrails-core+unsubscr...@googlegroups.com>. > To post to this group, send email to rubyonrails-core@googlegroups.com > <mailto:rubyonrails-core@googlegroups.com>. > Visit this group at https://groups.google.com/group/rubyonrails-core > <https://groups.google.com/group/rubyonrails-core>. > For more options, visit https://groups.google.com/d/optout > <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 > <mailto:rubyonrails-core+unsubscr...@googlegroups.com>. > To post to this group, send email to rubyonrails-core@googlegroups.com > <mailto:rubyonrails-core@googlegroups.com>. > Visit this group at https://groups.google.com/group/rubyonrails-core > <https://groups.google.com/group/rubyonrails-core>. > For more options, visit https://groups.google.com/d/optout > <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.
