Skip to content

Conversation

@waspper
Copy link

@waspper waspper commented Jul 30, 2020

No description provided.

Copy link

@andypost andypost left a comment

Choose a reason for hiding this comment

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

It needs split on commits for event rename and message link

Please exclude ETM rename as it just makes noise and will make hard to to make patch commitable to upstream

* The event to process.
*/
public function onKernelTerminate(PostResponseEvent $event) {
public function onKernelTerminate(TerminateEvent $event) {

Choose a reason for hiding this comment

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

Please file separate issue for that, also it needs link to change record where the event was renamed

*/
protected function entityTypeHasBundles($entity_type_id) {
$entity_type = $this->entityManager->getDefinition($entity_type_id);
$entity_type = $this->entityTypeManager->getDefinition($entity_type_id);

Choose a reason for hiding this comment

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

Entity manager addressed in d9-fixes branch where I did split commits on per d-org issue

Choose a reason for hiding this comment

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

Basically rename of the variable is useless at the moment

$message = ($status == SAVED_UPDATED) ? '%label event type updated.' : '%label event type added.';
$url = $event_type->toUrl();
$t_args = ['%label' => $event_type->id(), 'link' => Link::fromTextAndUrl(t('Edit'), $url)];
$t_args = ['%label' => $event_type->id(), 'link' => (Link::fromTextAndUrl($this->t('Edit'), $url))->toString()];

Choose a reason for hiding this comment

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

Good catch, also it needs separate issue (probably it exists)

Moreover it needs to rework messages as they are not translatable yet

Copy link

@andypost andypost Jul 30, 2020

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants