Problem/Motivation

Somehow #3280951: Drupal 10 compatibility did not include the composer.json to make this actually installable. An option would also be to remove it and let drupal.org fill it in automatically, then it only needs to be changed in one place.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork csv_serialization-3294354

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

Berdir created an issue. See original summary.

Berdir opened merge request !9

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Looks like 2.x and 3.x have no default configuration set up and the configuration for 1.x is outdated and for D8, which can no longer be tested.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

This makes sense!

diegoluisr made their first commit to this issue’s fork.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Most if not all of these changes are unnecessary/unrelated, you should not mix in coding standards improvements with other changes. For example, it might make sense to apply those changes to other branches too and it's easier to cherry-pick them if done separately.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Does this need to go back to "Needs work"? @Berdir's simple commit should be all that's necessary to resolve this issue.

@Diego Restrepo - can you open a new issue with your other changes so that this can be merged faster?

heddn made their first commit to this issue’s fork.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs work » Needs review

Reverted the code standard changes. We're now back to simply the drupal core version range changes in composer.json.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Reviewed & tested by the community

Manually tested this module on a Drupal 10.0.1 site. No errors. +1 on merging and tagging a Drupal 10 release.

andrewbelcher’s picture

andrewbelcher commented
Status: Reviewed & tested by the community » Needs work

3.x contains changes to signatures of methods extending symfony classes. It isn't compatible with the version of symfony supported by D9, so 3.x needs to be D10 only.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Did you try that? we discussed that in the linked issue and many but not all of these changes are forward compatible, meaning classes can be more strict than interfaces.

voleger made their first commit to this issue’s fork.

voleger opened merge request !13

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

It makes sense to delete the line and get rid of the need to synchronize the info.yml file with the composer.json file for now. Added MR with line removal.
When #3005229: Provide optional support for using composer.json for dependency metadata will be in, we can move the core version requirement into composer.json.

pwolanin’s picture

pwolanin commented

Isn't it better to remove this requirement from composer.json?

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

+ 1 for removal. See MR !13

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs work » Needs review

Both MRs needs review and the final decision on what needs to be made with the problematic line of the composer.json file.

markdorison’s picture

markdorison
he/him
commented
Status: Needs review » Reviewed & tested by the community

Merging MR !13. Thank you to everyone in this issue for testing and providing great feedback!

  • markdorison committed 9cc28b01 on 3.x authored by voleger 
    Issue #3294354 by Berdir, voleger, heddn, diegoluisr, andrewbelcher,...
markdorison’s picture

markdorison
he/him
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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