Automated Drupal 10 compatibility fixes

Patch works fine. Hiding files, so can concentrate on MR.
Also added contrib HAL module as dependency.

To test on Drupal 10

1. Add repository to composer.json before main drupal repository.

  "repositories": [
    {
      "type": "vcs",
      "url": "https://git.drupalcode.org/issue/better_normalizers-3286221.git"
    },
...

2. Run

composer require drupal/hal 
composer require drupal/better_normalizers:dev-3286221-8.x-1.x-automated-drupal-10

Is there a reason for the module in terms of D10?

I bet all normalizers already in core so no need in the module, even default content module v1 is not going to be compatible with D10

You are right Andrey
#3331814: Remove the Better Normalizers module from Varbase Core 10.0.x branch

No further needs to use

• A file normalizer that base64 encodes data, taken from file_entity.
• A menu link content normalizer which embeds dependencies on target entity, by UUID, when applicable.

Varbase Default Content and demo and testing content modules had been switched from base64 data to physical file under the new Default Content YAML format. and dropped the use of JSON in Varbase Default Content modules

My actions are:

• Remove the Better Normalizers module from Varbase Core 10.0.x branch
• Not to be enabled when exporting default content ( No longer needed )

Having a working module ( with composer update) in an upgrade step from Drupal 9 to Drupal 10. Is just a stage to ease up the upgrade process with auto updater.

I worked on #3160146: Add a Normalizer and Denormalizer to support Layout Builder
If this is in Drupal 10 core that will be so nice too

Hoping for a soft commit, and a soft release ( pre-release ), would speed up testing upgrades to Drupal 10

I think better to update project page with this notes (thank you)
Hal will need new major release 2.x branch

Is there a reason for the module in terms of D10?

I bet all normalizers already in core so no need in the module, even default content module v1 is not going to be compatible with D10

@andypost and/or other maintainers, could we get confirmation that this is the case? I don't understand the comment about default content module, it doesn't seem to depend on better_normalizers 🤔.

I think better to update project page with this notes (thank you). Hal will need new major release 2.x branch

It would be great to get the project page updated to the effect of whatever should be done for folks having this module as a dependency in Drupal 9 that are moving to Drupal 10. See also other issue recently opened related to Drupal 10 compatibility: #3354893: Deprecation warning 'Not marking service definitions as public is deprecated'.

Given all that, it doesn't seem like this is in actual fact RTBC, so updating the status.

Just added HAL as a dependency on composer and I brought the stuff from #3354893: Deprecation warning 'Not marking service definitions as public is deprecated' to the MR.

@andypost and/or other maintainers, could we get confirmation that this is the case? I don't understand the comment about default content module, it doesn't seem to depend on better_normalizers 🤔.

Great! I'm also have the same question and want to know it, I'm upgrading my project to D10 and I don't know if I can get rid off this module or not.

I'll move this back to NR to see what you guys think.

I have checked MR1 and in drupal-check getting one error which is not ignorable. So, uploading a patch and drupal-check error file for the same.

This is patch #20 rerolled to apply to beta5.

Thanks for sharing the report @muskan_mahajan, I've just updated the MR

I have tested MR !1 in drupal 10.0.9. It is working fine for me.
Moving to RTBC.

Hopping for a commit and a tag release

Only for testing:
Having the issue fork in the composer.json file.
In the repositories

  "repositories": {
    "drupal": {
      "type": "composer",
      "url": "https://packages.drupal.org/8",
      "exclude": [
        "drupal/better_normalizers"
      ]
    },
    "drupal/better_normalizers": {
      "type": "git",
      "url": "https://git.drupalcode.org/issue/better_normalizers-3286221.git"
    },
  },

and in require

  "require": {
    "drupal/core": "~10.0",
    "drupal/better_normalizers": "dev-3286221-8.x-1.x-automated-drupal-10 as 1.0-beta5",
  },

Still not clear what to commit MR vs patch, as next beta6 is needed for upgrade I'm fine to commit and create release but then it need to fix tests...

So what is a plan?

Hey @andypost I think you should go with the MR in this case. The MR have the test fixed(both for D9 and D10), and also incorporates the #3354893: Deprecation warning 'Not marking service definitions as public is deprecated'(which can be closed as duplicated since the deprecation notice reported is for D10 and we can address this here), what do you think?

Posting the static patch because using the MR doesn't allow pinning to a specific commit.

MR looks good to me.

+1 RTBC, can we get a new release?

Attached a patch that will apply cleanly to the d.o. packaged version of 8.x-1.0-beta5. This can be used to patch the module via composer. Be aware that if you want to update to D10 via composer you'll need to use one of several available workarounds to deal with the fact that composer cannot patch in a dependency change.

Works on D10 with the following in composer.json:

"require": {
  "drupaloverride/better_normalizers": "dev-master",
},
"repositories": [
  {
    "type": "package",
    "package": {
      "name": "drupaloverride/better_normalizers",
      "version": "dev-master",
      "type": "drupal-module",
      "dist": {
        "url": "https://git.drupalcode.org/api/v4/projects/project%2Fbetter_normalizers/repository/archive.zip?sha=312e16",
        "type": "zip"
      }
    }
  }
]

+1 to commit this

Hoping for a commit and a release.
To have a smooth upgrade process for projects.

+++ b/src/Normalizer/FileEntityNormalizer.php
@@ -38,7 +38,7 @@ class FileEntityNormalizer extends ContentEntityNormalizer {
-  public function normalize($entity, $format = NULL, array $context = array()) {
+  public function normalize($entity, $format = NULL, array $context = array()) : float|array|int|bool|\ArrayObject|string|null  {

I don't think this will be compatible with D9, so we will need a new major version here

This doesn't apply to the HEAD of 8.x-1.x can we get a reroll?

Thanks

MR is mergable.
If you are talking about the latest patch, it is for latest stable release, not for the dev.

Attached a new patch that will apply to HEAD.

Again to clarify:

#31 applies cleanly to the distributed version of 8.x-1.0-beta5, and is suitable for use in composer patches.

#38 applies to HEAD, and should be suitable to merge.

Thanks!

Cutting a 2.0.0-beta1

Please note that there will not be a version that works on D9 and D10, that's not possible because of the return type hint change for normalizers between Symfony 4 and 6.

I'd also like to echo the sentiment of @andypost above - you should be moving away from this module towards normalizers in core and default content v2

Thnaks 🍰