Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We missed converting one the variable_get('error_level') in the original conversion. This patch fixes this bug.
Part of #1775842: [meta] Convert all variables to state and/or config systems
Comment | File | Size | Author |
---|---|---|---|
#9 | 1798756-9.error_level.patch | 2.98 KB | alexpott |
#9 | 1798756-9.error_level.test-only.patch | 2.34 KB | alexpott |
#6 | 1798756-error_level_2.patch | 657 bytes | andreiashu |
#2 | 1798756-error_level_1.patch | 688 bytes | andreiashu |
Comments
Comment #1
andreiashu CreditAttribution: andreiashu commentedSpoke with @alexpott and it seems this has already been addressed in a separate issue but we discovered one place where it was missed. Patch to come
Comment #2
andreiashu CreditAttribution: andreiashu commentedpatch attached
Comment #3
andreiashu CreditAttribution: andreiashu commented@alexpott: does it make sense to have to default to a value at code level? We already have a default in system.logging.yml
Comment #4
andreiashu CreditAttribution: andreiashu commentedComment #5
andreiashu CreditAttribution: andreiashu commentedtagging
Comment #6
andreiashu CreditAttribution: andreiashu commentedRemoved the redundant default value
Comment #7
alexpottLooks great.
Thanks for the work!
Comment #8
webchickJust a quick question. Is this actually causing a bug to appear somewhere? If so, we need tests to illustrate the failure.
Comment #9
alexpottYep webchick you're right we need a test...
So here it is. The .test.only.patch should fail :)
Comment #11
alexpottSetting back to RTBC as the added test proves the bug and the fix!
Comment #12
aspilicious CreditAttribution: aspilicious commentedLooks good
Comment #13
Lars Toomre CreditAttribution: Lars Toomre commentedSmall nit going forward.... please do not use t() around assert messages. Thanks.
Comment #14
alexpottLars that t() is in all the ErrorHandlerTest's - copy/paste mistake :)
Comment #15
Lars Toomre CreditAttribution: Lars Toomre commentedThere are patches waiting review/commit removing all of the t() from assert messages. Hopefully, soon there won't be bad examples going forward.
Comment #16
webchickAwesome, thanks a lot!
Committed and pushed to 8.x.
Comment #17
Lars Toomre CreditAttribution: Lars Toomre commentedFor reference, this is the issue cleaning up t() around assert messages in the referenced test file: #1797926: Remove t() from asserts from System sub-system tests.
Comment #18
andreiashu CreditAttribution: andreiashu commented@alexpott: re #7: thanks for the guidance and helping out :)
Comment #19
andypostYep, one t() added with commit
this place should be fixed in #1797926: Remove t() from asserts from System sub-system tests
Comment #20.0
(not verified) CreditAttribution: commentedUpdated issue summary.