Problem/Motivation

Diff 2.x has return types added to FieldDiffBuilderBase::build.

This will fix 2.x support and not break 8.x-1.x support (since adding return types on child classes does not break compatibility).

Issue fork smart_date-3463447

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

acbramley created an issue. See original summary.

acbramley opened merge request !138

acbramley’s picture

acbramley commented
Status: Active » Needs review
mandclu’s picture

mandclu commented

Thanks for this! Any chances you could provide steps to reproduce?

acbramley’s picture

acbramley commented

@mandclu I've never used this module but I maintain the Diff module. I had similar issue with another module so I've created issues for all modules extending this class.

You'd just need to install Diff 2.x and diff something using this module's field type.

frankied3’s picture

frankied3
he/him
Primary language English
Location USA, UTC-4/5
commented

I can confirm the above. With the Smart Date and Diff modules enabled:

  1. Add a Smart Date field to any given content type
  2. Create a node of the content type with the Smart Date field
  3. Update the node to create a revision - note that you don't have to fill out the Smart Date field
  4. Compare the new revision using the Diff module through the node's revision page
micahw156’s picture

micahw156
he/him/his
commented

I can confirm with above. I had to do the same fixes on one of our custom modules earlier today.

pearls’s picture

pearls commented
Related issues: +#3480456: Fatal error occurs

+1 for RTBC.
Thanks @acbramley.

However, the same problems occur in Address, Dynamic Entity Reference, Office Hours modules. #3480456-Fatal error occurs
Maybe other modules that I don't know about also have this problem.

Wouldn't it be better to approach the issue through the Diff module?
Because it may be necessary to integrate a separate solution for each different module.

mandclu’s picture

mandclu commented
Status: Needs review » Fixed

Declaring return types is a best practice, and may soon be officially declared as a requirement in core. Thanks @acbramley for identifying this and posting a fix. Merged in.

micahw156’s picture

micahw156
he/him/his
commented
Version: 4.1.x-dev » 4.2.x-dev
Status: Fixed » Reviewed & tested by the community

Hi,

This patch was applied to the 4.1.x branch, but it's not included in the 4.2.0 release. Fortunately, the patch still applies fine for me against that release.

I've reopened the issue so this can get fixed in the current branch.

mandclu’s picture

mandclu commented
Status: Reviewed & tested by the community » Fixed

Thanks for catching that @micahw156. This has now also been applied to the 4.2.x branch, and I will roll a new release shortly.

micahw156’s picture

micahw156
he/him/his
commented

Thank you, @mandclu!

Status: Fixed » Closed (fixed)

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