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

[DoctrineBundle] document override_url #15321

Closed
wants to merge 1 commit into from
Closed

[DoctrineBundle] document override_url #15321

wants to merge 1 commit into from

Conversation

dbu
Copy link
Contributor

@dbu dbu commented May 9, 2021

I am targetting this branch because it is the oldest maintained one and the documentation is about a 3rd party bundle that can be installed with older symfony versions. i hope this is the right place.

doctrine/DoctrineBundle#1335 might change this some more but is not merged nor released yet. if it gets merged, the documentation needs to be completely overhauled for DoctrineBundle 2.4

@@ -128,6 +130,17 @@ The following block shows all possible configuration keys:
yet, you may get ``PDOException`` errors because Doctrine will try to
guess the database server version automatically and none is available.

.. note::
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ci complained about .. versionadded:: 2.3.

i just do a note as it seems we don't want versionadded for 3rd party components.

@@ -50,7 +50,8 @@ The following block shows all possible configuration keys:
user: user
password: secret
driver: pdo_mysql
# if the url option is specified, it will override the above config
# until doctrine-bundle 2.2, the url option overrides the above config
# since doctrine-bundle 2.3 it is recommended to set override_url: true, which allows the above config to override the parts of the url
Copy link
Member

Choose a reason for hiding this comment

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

Actually not, as we're going to deprecate override_url

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are you reverting the feature? or will you do doctrine-bundle 3 and force the behaviour?

any suggestion what to say here?

Copy link
Member

Choose a reason for hiding this comment

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

We are going to revert the feature yes. Maybe we should just keep the existing sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, then should i change the note to say that DoctrineBundle 2.3 introduced override_url but the feature will be retired and recommend to set override_url to false?

also, if you will revert anyways, why not change the recipe to not set override_url (or set it to false)?

Copy link
Member

Choose a reason for hiding this comment

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

we merged the change yesterday :) symfony/recipes#939
maybe there is nothing to do...

@dbu
Copy link
Contributor Author

dbu commented May 10, 2021

okay yeah then no point in having this here. there will be a bit of confusion for a while still i guess, with people who recently installed DoctrineBundle, but imo no point having this in the documentation.

thanks for the feedback and your work!

@dbu dbu closed this May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants