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

Twig: adjust version check to not depend on changing VERSION_ID constant #361

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

glaubinix
Copy link
Contributor

@glaubinix glaubinix commented Sep 10, 2024

Twig versions are defined like public const VERSION_ID = 31500; see https://github.com/twigphp/Twig/blob/3.x/src/Environment.php#L47 there is no 0 between major and minor

@pscheit
Copy link
Contributor

pscheit commented Sep 10, 2024

mhhh, but there was:
twigphp/Twig@221e62c#diff-a649e8fcd77d506cd5ef3e00f495e87c0583b3bdee1ea0e953d9f9c2681a39e2R47

So i would suggest to not use the VERSION_ID then but to compare major + minor?

Copy link
Contributor

@pscheit pscheit left a comment

Choose a reason for hiding this comment

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

just some feverish thoughts

src/Twig/Version.php Outdated Show resolved Hide resolved
@glaubinix glaubinix changed the title Twig: fix version check by removing additional 0 Twig: adjust version check to not dependend on changing VERSION constant Sep 10, 2024
@glaubinix glaubinix changed the title Twig: adjust version check to not dependend on changing VERSION constant Twig: adjust version check to not dependend on changing VERSION_ID constant Sep 10, 2024
@glaubinix glaubinix changed the title Twig: adjust version check to not dependend on changing VERSION_ID constant Twig: adjust version check to not depend on changing VERSION_ID constant Sep 10, 2024
@Seldaek Seldaek merged commit 3c47396 into nelmio:master Sep 10, 2024
@glaubinix glaubinix deleted the patch-1 branch September 10, 2024 13:25
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