[bootstrap_layouts] Drupal 9 support

Removing outdated tag.

I fixed these error reported by drupal-check:

 19/19 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ---------------------------------------------------------------------- 
  Line   src/BootstrapLayoutsManager.php                                       
 ------ ---------------------------------------------------------------------- 
  271    Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
  274    Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
  275    Call to deprecated function drupal_set_message():                     
         in drupal:8.5.0 and is removed from drupal:9.0.0.                     
         Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.  
 ------ ---------------------------------------------------------------------- 

 [ERROR] Found 3 errors                                                         

+1
I have sent the maintainer sdarol100 and markcarver to urge a commit/release.

There were some issues with the previous patch. I believe I've addressed them.

Please review and test with an actual site/install, then set back to RTBC once you've verified it actually works.

I'll create a new release shortly after.

Patch applied from #7 looks good to me.

19/19 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors

Commenting the output a CLI command isn't verification.

Hi @markcarver, I see that you have reopened the issue. Can you explain what was the reason?

Did you actually check if both real D8 and D9 installs work?

Hi @markcarver, as per the scope of the issue here it was to remove deprecated code that shows up as error when you run upgrade status and drupal-check. And running both the tools show that there are no more deprecation errors so it is good to be deemed compatible with D9 and hence we have the core_version_requirement: ^8 || ^9 to bootstrap_layouts.info.yml to designate that the module is compatible with Drupal 9 hence the issue can be closed as I have tested the patch and it applies correctly.

That isn't what I asked.

@markcarver as per the tools that we use: upgrade status and drupal-check, these tools are used mainly to check which code is deprecated and incompatible with D9. The main idea is that if these tools don't throw any error so your code would be compatible with D9. There is no need to manually test it separately as there is no major upgrade changes from D8 to D9 hence these tools should be enough to tell us that.

If in the rare case that we still see issues with the module in D9 then we can create separate issue for that.

@markcarver applied patch#7, installed it successfully on d8.8.7 and d9.0.0.

The main idea is that if these tools don't throw any error so your code would be compatible with D9.

This is the primary issue I have with tools like this: they're only as good as they were made. Meaning: they only catch very specific rules that they're programmed with. These rules can and often change over time.

In the case of these "deprecations", they are also only as good as "this replaced this". It doesn't mean that it is always the best solution, nor do these tool check for syntax/logical errors that may have been introduced by patches.

That's why I asked for actual testing to ensure that the patch I had originally supplied in #7 was technically working. Thanks @divyesh19 BTW!

I currently don't have any instances of this module spun up. I just made the quick changes based on the previous patch that was submitted (#4) and the few issues I saw with it. I needed legitimate, real-world verification and feedback; not some explanation about how you're using CLI automation tools... I immediately got that from the output you posted; not what I asked for nor needed.

---

That all being said, after working on #3096963: [bootstrap] Drupal 9 support for Bootstrap:

+++ b/bootstrap_layouts.info.yml
@@ -2,6 +2,8 @@ name: Bootstrap Layouts
+  - drupal:system (>=8.5.0)

I'm not comfortable with this change. This kind of hard dependency bump should be left to major version bumps, not minor once. Considering that the deprecated code here is pretty much just limited to the messenger service, I'd rather just throw in a simple if statement that can support both.

Here's a much simpler and more appropriate approach to this issue than simply requiring newer APIs and removing the deprecated implementations.

thanks for the insights @markcarver, makes sense.
applied #19 cleanly, and installed it on both D8 and D9. works fine!

Thanks, @divyesh19!

This is also technically a feature request.