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

Fix version information #7957

Merged
merged 2 commits into from
Dec 17, 2019
Merged

Conversation

lcobucci
Copy link
Member

Since Doctrine\ORM\Version is now deprecated it shall not be updated on future releases.

This ensures that our CLI tool will present the correct version number.

We aren't using ant/phing to handle the releases any more.
@lcobucci lcobucci added this to the 2.7.1 milestone Dec 17, 2019
@lcobucci lcobucci self-assigned this Dec 17, 2019
@lcobucci lcobucci force-pushed the fix-version-information branch 2 times, most recently from c5dc4f0 to bfc11a4 Compare December 17, 2019 00:15
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Looks good, I added suggestions to fix the build

lib/Doctrine/ORM/Tools/Console/ConsoleRunner.php Outdated Show resolved Hide resolved
Since `Doctrine\ORM\Version` is now deprecated it shall not be updated
on future releases.

This ensures that our CLI tool will present the correct version number.
@lcobucci lcobucci merged commit 2785cde into doctrine:2.7 Dec 17, 2019
@lcobucci lcobucci deleted the fix-version-information branch December 17, 2019 18:19
@alcaeus
Copy link
Member

alcaeus commented Dec 18, 2019

Since Doctrine\ORM\Version is now deprecated it shall not be updated on future releases.

This is a significant BC break: the only purpose of the class is to expose the currently installed version so it be updated on subsequent releases.

Since the version is exposed as a constant, there's no easy way to dynamically generate this. A workaround could be to create the class dynamically in an autoloader file (reading the version with the help of the ocramius/package-versions package), but that could be considered hacky. @lcobucci would you be open to merging that solution?

@Ocramius
Copy link
Member

This is a significant BC break: the only purpose of the class is to expose the currently installed version so it be updated on subsequent releases.

It's not a BC break: the version is frozen, and new consumers (relying on version_compare()) will immediately see the deprecation. Old consumers will be fine (no changes there).

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.

4 participants