Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable doctrine/dbal dev job #9819

Closed
wants to merge 1 commit into from

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Jun 5, 2022

Porting prep work to 2.12.x would mean porting fixes for something that
is not actually impacting the end user.

Porting prep work to 2.12.x would mean porting fixes for something that
is not actually impacting the end user.
@greg0ire greg0ire requested review from derrabus and morozov June 5, 2022 14:58
@morozov
Copy link
Member

morozov commented Jun 5, 2022

I see that ORM 2.12.x supports DBAL ^3.2:

"doctrine/dbal": "^2.13.1 || ^3.2",

What will happen after the DBAL 3.4.0 release?

@greg0ire greg0ire marked this pull request as ready for review June 5, 2022 18:46
@greg0ire
Copy link
Member Author

greg0ire commented Jun 5, 2022

Didn't think of that... I think maybe we should release ORM 2.13 first.
🤔 ah but ORM 2.13 requires DBAL 3.4 … maybe the solution is to add use the conflicts section of the DBAL's composer.json

@morozov
Copy link
Member

morozov commented Jun 5, 2022

maybe the solution is to add use the conflicts section of the DBAL's

The DBAL doesn't depend on the ORM nor does it provide any of the APIs provided by the ORM, so it cannot conflict with it.

In general, I don't think the introduction of a conflict is the best idea because:

  1. It would imply that they are incompatible and thereby the DBAL has introduced a BC break which it didn't.
  2. Users would have to update both the DBAL and the ORM at the same time which should be avoided, if possible.

A better solution might be to:

  1. Remove the tests that were removed in 2.13.x.
  2. For the rest of failing tests that check if ($platform->supportsForeignKeyConstraints()) also check if (!$platform instanceof SqlitePlatform).

@derrabus
Copy link
Member

derrabus commented Jun 6, 2022

In general, I don't think the introduction of a conflict is the best idea because:

  1. It would imply that they are incompatible and thereby the DBAL has introduced a BC break which it didn't.

  2. Users would have to update both the DBAL and the ORM at the same time which should be avoided, if possible.

For the same reasons, I'd like to avoid removing the @dev workflow. The ORM's functional tests are a pretty good indicator for potential BC breaks inside the DBAL. If our tests are broken, we should rather fix the tests.

@greg0ire greg0ire closed this Jun 6, 2022
@greg0ire greg0ire deleted the disable-dbal-dev-job branch June 6, 2022 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants