Automated Drupal 10 compatibility fixes

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: 📌 Deprecation warning 'Not marking service definitions as public is deprecated' Needs review .

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 📌 Deprecation warning 'Not marking service definitions as public is deprecated' Needs review 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 want to know this, 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.

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

I checked Merge Request 1 but 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 📌 Deprecation warning 'Not marking service definitions as public is deprecated' Needs review (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 releas.
To have a smooth upgrade 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 release, not the head.

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 📌 Automated Drupal 10 compatibility fixes RTBC , 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 🍰

Automatically closed - issue fixed for 2 weeks with no activity.