The trigger_error php function should not be used, instead use the logger from the framework. I'm providing a MR shortly.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3218693-4.patch | 3.31 KB | bbrala |
Issue fork jsonapi_extras-3218683
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
jurgenhaasComment #4
bbralaThe line is pretty much a copy of the code in core. I've also checked the code of those 2 methods and i actually think the methods
ConfigurableResourceTypeRepository::getRelatableResourceTypesFromFieldDefinitionandConfigurableResourceTypeRepository::lookupResourceTypemight actually be redundant since #2996114: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given has been merged. The code that was placed in extra's has been migrated to core.I've attached a patch to test that assumption.
Comment #5
bbralaOk i just realized, the patch was not merged into 8.9.x, unfortunate.
Comment #7
bbralaThanks for your merge request. :)
I'm not too sure i want to diverge from the core solution too much. In core it's also a trigger error. When i remove this code when Drupal 8 support is dropped this change to use logger will result in changes in behaviour. I'd prefer we don't do that.
So unless you have compelling reasons to apply this anyways i'd rather not.
Comment #8
jurgenhaas@bbrala thanks for your reply. The use case is pretty generic: we have monitoring in place which listens to all the standard logging channels that Drupal uses. That's the logger service almost everywhere and logger optionally stores the log messages in either dblog or syslog. We also have analysis tools that are attached to the logger service and they qualify the log message by certain rules and trigger an alert in a devop platform, where the IT team gets notified and can react as required.
However, you're right that the two functions could be removed completely, then this is not an issue of jsonapi_extras anymore.
Comment #9
bbralaFair enough, i understand your need. But Drupal 8 EOL is pretty soon, so around that time (or a little earlier) a new major release will be published. This will remove these lines from the code :) Until then i won't be changing the code compared to core.
You are ofcourse free to use the patch you supplied in your projects ;)
Comment #11
bradjones1Rebased. This is no longer postponed as D8 is bye-bye.
Comment #12
jurgenhaasLooks like this has made it to the latest release? I just figured that the patch doesn't apply anymore and the 2 functions have disappeared from the codebase. Maybe just the reference to this issue was missing in the commit message, never mind.
Comment #13
bbralaAhh, didn't find this issue, I removed old code paths when preparing the release. These were indeed removed.
Fixing and giving credit, even if it wasnt committed through this issue.