Replace usages of the deprecated drupal_set_message() function

Hi @Prashant.c
Your patch applied cleanly and it removed drupal_set_message by messenger service.Please refer the attached screenshot.

New patch to format correct PHP comments.

Hi, @romainj and @eme as this is a change introduced in Drupal 8.5 I think that we need to make a decision.

We have users that can have a Drupal version < 8.5 and can make a module update to the latest version that will include this patch, in this case, the module will not works. I see two possible solutions, create a new branch 8.x-2.X or create a new release and add a note about the minimal Drupal Core version.

I'd say that we should open a new branch. BUT we will possibly have to maintain the two for a long time which is hard work.
So is this patch really needed?

@romainj this will really matter in Drupal 9.0.0 as the function docblock now says:

* @deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0.
* Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.

We can keep the function and add a @TODO to the place where it used.

I'm trying to find others opinions in How to handle a core change that have an impact in your next release?

@romainj this is an answer on Slack from @berdir:

@adriancid there is no need for a new branch in your module as long as your own API doesn't change IMHO

the version definition will prevent that users update too early
but there is no need to continue providing versions compatible with old core versions, they are no longer supported
and if it's just about deprecated usages, there's no reason to hurry, you can easily wait a few month/next core minor before starting to use it to give your users time to update. It's different if core broke something or if there is a completely new API that you'd want to use

So I think that we can go ahead with this as once a new Core version is out the latest will not longer receive updates, so with this change we need to add to the info.yml file a core dependency like:

dependencies:
 - system (>=8.5)

So here is the new patch :-) and the interdiff.

Another update because an Interface should be used instead of a class when you're injecting services.

Interdiff between #5 and this patch is provided

I think it is reasonable to wait until Drupal 8.6 so that everyone got enough time to update. Could we add a hook_requirements() to prevent upgrading the Admin Toolbar when using a Drupal version prior to 8.5?

We really can commit this to the dev branch as we don't plane to made a release for the moment, and we can create a new release once the Drupal 8.6 version will be available.

@adriancid Yes you are right we have no rush to release a new version. Still I would prefer not to integrate the patch in the dev version now so that in case of a critical bug arises we are able to release a version without that patch. In any case we will take care of this issue as soon as we think it's time to.

We need to create a new patch before adding this functionality to the module because the ToolbarController class extends from ControllerBase class and in this class, we have the MessengerTrait trait, so we don't need to inject the service in ToolbarController, we need to use the MessengerTrait.

Similar issue in #2969630: Use the MessengerTrait instead of inject the messenger service

@romainj when do you think that we can put this into the dev version?

@adriancid I think that we should keep the actual 1.23 version compatible with all version of Drupal core and release a version 2 of Admin Toolbar compatible with Drupal core 8.5 and higher. As far as I know not every Drupal 8 sites upgrade to 8.5 version. So we don't want them to break when updating the Admin Toolbar.

@adriancid the patch #10 does not apply anymore.

The message when clearing all caches does not appear anymore.

Hi @romain, as ToolbarController extends ControllerBase, and ControllerBase uses the MessengerTrait trait, we don't need to inject the messenger service.

Hi @romainj with the new Drupal 8.5 do you think that is time to add this patch to the module oo we need to wait more?

@adriancid Yes I also think that it's time to commit the patch.