search

liufengyun / hashdiff

Hashdiff is a ruby library to to compute the smallest difference between two hashes
MIT License
559 stars 63 forks source link

Please stop the `warn` spam #66

Closed grosser closed 5 years ago

grosser commented 5 years ago

now every test run/boot has a warning :( Just remove the constant and be done with it ... or do whatever activesupport does but only when a deprecated constant is used

caused by https://github.com/liufengyun/hashdiff/pull/65

jfelchner commented 5 years ago

"spam" is something that is useless. Warning users about a deprecation is not useless. Is it intrusive? Yes. Is it a potentially huge issue if people don't update their constants before they upgrade? Also yes.

The warning intrusion is in proportion to the impact of not heeding it.

grosser commented 5 years ago

Warnings I cannot do anything and already know about, are spam for me. Whoever depends on version >=0 should be prepared for things to break, using ~> 0.3 would be safe.

To reduce blast-radius: see which gems depend on hashdiff and reach out to them, whoever uses it in their tests directly should see the blow-up To warn on use: define a proxy with the old name and use method_missing to warn+delegate

On Mon, Jun 3, 2019 at 12:26 PM Jeff Felchner notifications@github.com wrote:

"spam" is something that is uselss. Warning users about a deprecation is not useless. Is it intrusive? Yes. Is it a potentially huge issue if people don't update their constants before they upgrade? Also yes.

The warning intrusion is in proportion to the impact of not heeding it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/liufengyun/hashdiff/issues/66?email_source=notifications&email_token=AAACYZ22M5NI5GGNRY6ITUTPYVV6FA5CNFSM4HR4C32KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2N4ZI#issuecomment-498392677, or mute the thread https://github.com/notifications/unsubscribe-auth/AAACYZ5XN7WZDW3MUDJIJW3PYVV6FANCNFSM4HR4C32A .

grosser commented 5 years ago

thx for being so careful though, I did not know what depends on this gem ... I just use it for some api diffing tools :)

jfelchner commented 5 years ago

@grosser I completely agree. And we have reached out to gems that use this gem. Specifically the biggest one: webmock. But we can't be assured that we're going to get them all, hence the warning is still necessary.

webmock has always had an open-ended dependency on hashdiff (> 0.0) which means that we needed the warning in case there was any weird old combination of versions with old webmock installs.

I have PRs that should patch webmock versions 1.x, 2.x, and 3.x. Once those are in, we'll release a v1 of hashdiff and the warnings should go away for everyone.

A proxy would definitely work but is also potentially more error-prone. Because webmock is such a popular gem, I didn't want to take the chance that an error in the proxy implementation (no matter how simple a method_missing would be) would cause every one of the millions of webmock installs to fail.

grosser commented 5 years ago

Sounds like you are having a good time, good luck :D

On Mon, Jun 3, 2019 at 12:51 PM Jeff Felchner notifications@github.com wrote:

@grosser https://github.com/grosser I completely agree. And we have reached out to gems that use this gem. Specifically the biggest one: webmock. But we can't be assured that we're going to get them all, hence the warning is still necessary.

webmock has always had an open-ended dependency on hashdiff (> 0.0) which means that we needed the warning in case there was any weird old combination of versions with old webmock installs.

I have PRs that should patch webmock versions 1.x, 2.x, and 3.x. Once those are in, we'll release a v1 of hashdiff and the warnings should go away for everyone.

A proxy would definitely work but is also potentially more error-prone. Because webmock is such a popular gem, I didn't want to take the chance that an error in the proxy implementation (no matter how simple a method_missing would be) would cause every one of the millions of webmock installs to fail.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/liufengyun/hashdiff/issues/66?email_source=notifications&email_token=AAACYZ7GYBGGJ332ERU55PTPYVY2LA5CNFSM4HR4C32KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW2P44Q#issuecomment-498400882, or mute the thread https://github.com/notifications/unsubscribe-auth/AAACYZ425AYHLW6H6GLRUTDPYVY2LANCNFSM4HR4C32A .

liufengyun commented 5 years ago

Thank you all @jfelchner @grosser @krzysiek1507 , I close the PR then.