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

[FEATURE] Doctrine ORM 3.0 & DBAL 4.0 & Doctrine Coding Standard #582

Conversation

TomHAnderson
Copy link
Contributor

@TomHAnderson TomHAnderson commented Sep 30, 2024

Please prefix your pull request with one of the following: [FIX] [FEATURE].

Changes proposed in this pull request:

  • Works with Doctrine ORM 3 and DBAL 4
  • src conforms to the Doctrine Coding Standard
  • scripts for composer.json to run all tests and code coverage report

@TomHAnderson TomHAnderson changed the title Feature/doctrine coding standard - extend feature/orm3 [FEATURE] Doctrine ORM 3.0 & DBAL 4.0 & doctrine coding standard Sep 30, 2024
@TomHAnderson TomHAnderson changed the title [FEATURE] Doctrine ORM 3.0 & DBAL 4.0 & doctrine coding standard [FEATURE] Doctrine ORM 3.0 & DBAL 4.0 & Doctrine Coding Standard Sep 30, 2024
Comment on lines +24 to +25
"doctrine/dbal": "^4.1",
"doctrine/orm": "^3.1",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this is handled on this repository usually, but it might be smart to create releases that support ORM 2 and 3 first, so people can migrate one package at a time. Same for DBAL.

Copy link
Contributor Author

@TomHAnderson TomHAnderson Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ORM 3 requires DBAL 4. What my PR proposes is a major version release without a path to migrate "gracefully".

This library is sorely lacking support for the latest Doctrine libraries. If version 2 of this library still works for folks, that's fine. But constant progress on this library is non-existent. It's like the whole team (10 of you?) has dropped their assumed goal of maintaining a world-class library.

It's not uncommon for older packages to lose traction after some time, but this library is the best solution to Doctrine ORM in Laravel and both those projects continue to mature.

Copy link
Member

@eigan eigan Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's like the whole team (10 of you?) has dropped their assumed goal of maintaining a world-class library.

Note that the Doctrine team is not related to this library. I have been the sole "maintainer" of this library for several years. But I almost never contribute myself, it's up to the community to provide pull requests, unless none have provided a pull request before laravel goes out of security updates.

Yes, laravel-doctrine needs a proper maintainer which can lift this package up to latest standard and keep constant progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this isn't you:
https://github.com/laravel-doctrine/orm/blob/2.0/LICENSE#L3
And this isn't you:
https://github.com/laravel-doctrine/orm/blob/2.0/composer.json#L14-L15

I would like to be that proper maintainer for this organization. I have many Doctrine ORM and Laravel libraries at https://github.com/api-skeletons I am also a member of the Doctrine open source team. My latest project, the LDOG Stack, https://github.com/api-skeletons/ldog, relies on this library as well as my other projects. Reviving this resource is in my best interest.

To see my latest work please review https://github.com/api-skeletons/doctrine-orm-graphql (and be sure to see the documentation).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont feel that I have contributed enough to this repo that I feel I should be on those lists.. :)

I will take a look at your profile. I don't think it will be any problem of giving you access 👌

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ORM 3 requires DBAL 4.

It does not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see in this composer.json, libraries can be compatible with both ORM 2 and 3 and DBAL 3 and 4.
https://github.com/API-Skeletons/doctrine-orm-graphql/blob/12.2.x/composer.json#L14-L22

I don't foresee version 3 of this library trying to do the same. So I think to catch this library up, ORM 3 should be supported at the highest versions.

@TomHAnderson
Copy link
Contributor Author

@eigan Before I do anything within this organization, will you please email me?
tom.h.anderson@gmail.com

@TomHAnderson TomHAnderson changed the base branch from 3.0 to 3.0.x October 3, 2024 17:56
@TomHAnderson TomHAnderson force-pushed the feature/doctrine-coding-standard branch from e6f1e76 to 50f6bd1 Compare October 3, 2024 18:00
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 63.33333% with 231 lines in your changes missing coverage. Please review.

Please upload report for BASE (3.0.x@ee9f2e8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/DoctrineServiceProvider.php 0.00% 71 Missing ⚠️
src/Console/DumpDatabaseCommand.php 0.00% 19 Missing ⚠️
src/IlluminateRegistry.php 70.00% 12 Missing ⚠️
src/EntityManagerFactory.php 87.91% 11 Missing ⚠️
src/Notifications/Notification.php 0.00% 10 Missing ⚠️
src/Testing/Factory.php 47.36% 10 Missing ⚠️
src/Configuration/MetaData/Fluent.php 0.00% 7 Missing ⚠️
src/Testing/Concerns/InteractsWithEntities.php 53.33% 7 Missing ⚠️
src/Queue/FailedJobsServiceProvider.php 0.00% 6 Missing ⚠️
src/Validation/PresenceVerifierProvider.php 0.00% 6 Missing ⚠️
... and 32 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##             3.0.x     #582   +/-   ##
========================================
  Coverage         ?   64.04%           
  Complexity       ?      531           
========================================
  Files            ?       76           
  Lines            ?     1677           
  Branches         ?        0           
========================================
  Hits             ?     1074           
  Misses           ?      603           
  Partials         ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TomHAnderson TomHAnderson merged commit 3c26edd into laravel-doctrine:3.0.x Oct 10, 2024
14 of 17 checks passed
@TomHAnderson
Copy link
Contributor Author

Merging this without static analysis. Will be addressed in future PR

@TomHAnderson TomHAnderson self-assigned this Oct 11, 2024
@TomHAnderson TomHAnderson added this to the 3.0.0 milestone Oct 11, 2024
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.

4 participants