-
Notifications
You must be signed in to change notification settings - Fork 374
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.
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
Add more 2.x deprecation warnings #3647
base: 1.x-stable
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 1.x-stable #3647 +/- ##
===========================================
Coverage 98.24% 98.24%
===========================================
Files 1255 1255
Lines 74651 74659 +8
Branches 3544 3548 +4
===========================================
+ Hits 73342 73352 +10
+ Misses 1309 1307 -2 ☔ View full report in Codecov by Sentry. |
8f1430e
to
7ddccc5
Compare
I'd like to rebase this on the changes implemented in #3675, so that these deprecation warnings can utilize the limit feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM
7ddccc5
to
4b44893
Compare
I've rebased the changes on top of the "log limit" functionality backported via #3780. This should prevent verbosity on the configuration path. We could go further with changes here, but I think I want to stop here for now; I need to move onto other changes, and want to get this merged before it gets either too large or stale. Should be ready for review. |
Hey @delner is this still relevant? |
What does this PR do?
Adds some more deprecation warnings for changes outlined in the upgrade guide. Includes:
Use of non-strings for 'env' configuration is deprecated. Use a string instead.
Use of non-strings for 'service' configuration is deprecated. Use a string instead.
Motivation:
To help users more easily through the 2.x upgrade process. These warnings should make it easier for users to identify breaking changes on existing 1.x deployments, and update their code in a 1.x/2.x compatible way, before undertaking a 2.x upgrade.
Additional Notes:
There are many changes in the upgrade guide. Some may be worth warning about, others may be more trouble than they are worth. I'm mostly focusing on actionable.
We also should be careful with how verbose some of these warnings could be, especially when they are in the instrumentation path (and could be triggered many times.) Perhaps we should add some more explicit "print once" behavior?