TaggedHandlersPass is very hard to understand and while the class has useful error messages they are hidden. They are hidden because they only show up when the kernel.environment is not prod but we do not yet have a toggle for that and kernel.environment is always prod (aside from tests and update.php).
Do not run the if ($container->getParameter('kernel.environment') === 'prod') { continue; } chunks.
Figure out what to do with the code -- remove? comment out and do the same with the test.
None.
None. TaggedHandlersPass becomes slightly easier to use and that's a good thing because TaggedHandlersPass needs every bit of help it can towards usability.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | drupal8.taggedhandlers-prod.9.patch | 3.6 KB | sun |
| d8_thp_cruft.patch | 852 bytes | chx |
Comments
Comment #2
chx commentedSo apparently there's a test for this being covered. I have no idea what's next, comment out and add a @TODO to add back when (if ever) #1537198: Add a Production/Development toggle happens?
Comment #3
chx commentedComment #4
chx commentedComment #5
chx commentedComment #6
dawehnerOr we just wait until #1537198: Add a Production/Development toggle is in?
Comment #7
dawehnerI would be fine with adding a @TODO and remove the current code and comment out the test. It is indeed a pain to work something which don't give you errors at all.
Comment #8
sunFine to remove that code for now, including the test methods.
When this condition was introduced, a prod/dev environment flag was very actively discussed and backed by code; so it was not unrealistic to assume that it would happen, but then the issue was rethought. If it is going to happen at some point, we can consider to restore the conditions + tests (or perhaps also not).
Note that there are two instances of the 'prod' condition in
TaggedHandlersPass.Comment #9
sunAttached patch removes the conditions and the corresponding tests.
No test coverage is lost; the
$env = 'prod'tests were intentionally split into separate unit test methods.Comment #11
sun9: drupal8.taggedhandlers-prod.9.patch queued for re-testing.
Comment #12
chx commentedComment #14
sun9: drupal8.taggedhandlers-prod.9.patch queued for re-testing.
Comment #15
dawehnerGreat.
Comment #16
catchCommitted/pushed to 8.x, thanks!