Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🔊 Warn about deprecated #responses usage #97

Merged
merged 1 commit into from
Jun 23, 2024
Merged

🔊 Warn about deprecated #responses usage #97

merged 1 commit into from
Jun 23, 2024

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented Jan 7, 2023

This was extracted from #93 and split into a separate PR.

This allows us to prepare dependent projects for the new behavior before their logs are flooded with deprecation warnings.

Also adds Net::IMAP.attr_accessor :silence_thread_safety_deprecation_warnings. This is provided as a temporary workaround, until dependant projects can update their usage.

@nevans nevans requested a review from shugo January 7, 2023 15:30
@nevans
Copy link
Collaborator Author

nevans commented Jan 7, 2023 •
edited
Loading

@shugo Because #93 is such a big change to existing usage, I split the deprecation warnings into a separate PR. I'd like to take a look at the top reverse dependencies and submit forward-compatible PRs to them, before we start flooding their logs with new warnings!

@nevans nevans force-pushed the response-handlers branch from 51f57de to 8e63a33 Compare January 9, 2023 15:32
shugo
shugo previously approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@shugo shugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine!

@shugo
Copy link
Member

shugo commented Jan 10, 2023

FYI, warn(msg, category: :deprecated) doesn't show warnings by default.
However, Net::IMAP users may not use -W:deprecated....

@nevans
Copy link
Collaborator Author

nevans commented Jan 10, 2023 •
edited
Loading

FYI, warn(msg, category: :deprecated) doesn't show warnings by default. However, Net::IMAP users may not use -W:deprecated....

Thanks. I had forgotten about that.

If I remember correctly, I did try to use category: :deprecated on the deprecated SASL mechanisms, but I removed it because that kwarg wasn't supported in 2.6 or 2.7. When we do require ruby 3.0+, should we switch to use category: :deprecated?

@shugo
Copy link
Member

shugo commented Jan 11, 2023

If I remember correctly, I did try to use category: :deprecated on the deprecated SASL mechanisms, but I removed it because that kwarg wasn't supported in 2.6 or 2.7. When we do require ruby 3.0+, should we switch to use category: :deprecated?

Yes, you're right. I forgot that category: :deprecated was not supported in 2.7.

@nevans nevans force-pushed the response-handlers branch from 8e63a33 to 40a15db Compare June 12, 2023 17:46
@nevans nevans force-pushed the response-handlers branch 2 times, most recently from 857dabb to bbaa32a Compare August 5, 2023 02:56
@nevans nevans marked this pull request as draft August 29, 2023 14:09
@nevans nevans force-pushed the response-handlers branch 2 times, most recently from d800e82 to c58afb8 Compare September 26, 2023 01:25
@nevans nevans force-pushed the response-handlers branch from c58afb8 to 6cb82ff Compare October 4, 2023 19:59
@nevans nevans force-pushed the response-handlers branch 2 times, most recently from c835ad5 to ecba223 Compare November 7, 2023 04:28
@nevans nevans force-pushed the response-handlers branch from ecba223 to 0ccd250 Compare April 30, 2024 13:08
@nevans nevans force-pushed the response-handlers branch from 0ccd250 to 4c1f98e Compare May 19, 2024 14:09
@nevans nevans added the v0.5 label May 19, 2024
@nevans nevans force-pushed the response-handlers branch from 4c1f98e to 68ba96e Compare May 19, 2024 14:35
@nevans nevans added this to the v0.5 milestone May 19, 2024
@nevans nevans removed the v0.5 label May 19, 2024
@nevans nevans force-pushed the response-handlers branch from 68ba96e to ba5da45 Compare June 12, 2024 02:59
This was referenced Jun 12, 2024
Merged
Closed
Merged
@nevans nevans force-pushed the response-handlers branch 3 times, most recently from bd0ec61 to 7306c2c Compare June 15, 2024 02:23
@nevans nevans force-pushed the response-handlers branch 2 times, most recently from 1427a0f to 170a77d Compare June 16, 2024 14:05
@nevans nevans force-pushed the response-handlers branch from 170a77d to 78cd2fe Compare June 22, 2024 16:14
@nevans
🔊 Warn by default on deprecated responses usage
d4a8c54 
This can still be silenced via the config option.
@nevans nevans force-pushed the response-handlers branch from 78cd2fe to d4a8c54 Compare June 23, 2024 02:10
@nevans nevans merged commit d4a8c54 into master Jun 23, 2024
18 checks passed
@nevans nevans deleted the response-handlers branch June 23, 2024 02:28
nevans added a commit that referenced this pull request Sep 16, 2024
@nevans
🗑 Add ^category: :deprecated` to calls to warn
1da8ef9 
This was recommended by @shugo for #97, but we needed to support ruby
2.7 at that time.  The minimum required ruby is 3.1 now, so should
update our deprecation warnings.
@nevans nevans mentioned this pull request Sep 16, 2024
Merged
nevans added a commit that referenced this pull request Sep 16, 2024
@nevans
🗑 Add category: :deprecated to calls to warn
8168ba8 
This was recommended by @shugo for #97, but we needed to support ruby
2.7 at that time.  The minimum required ruby is 3.1 now, so should
update our deprecation warnings.
nevans added a commit that referenced this pull request Sep 16, 2024
@nevans
🗑 Add category: :deprecated to calls to warn
1bd57c8 
This was recommended by @shugo for #97, but we needed to support ruby
2.7 at that time.  The minimum required ruby is 3.1 now, so should
update our deprecation warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shugo shugo shugo left review comments

Assignees
No one assigned
Labels
None yet
Milestone
v0.5
Development

Successfully merging this pull request may close these issues.
2 participants
@nevans @shugo