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

PHP7 Compatibility release #16

Open
wants to merge 12 commits into
base: integration
Choose a base branch
from

Conversation

alexnantel88
Copy link

No description provided.

@alexnantel88
Copy link
Author

wait for @nitriques to merge it

@@ -14,6 +14,9 @@
</author>
</authors>
<releases>
<release version="1.7.2" date="2017-05-19" min="2.5.x" max="4.x.x" php-min="5.6.x" php-max="7.x.x">
Copy link
Member

Choose a reason for hiding this comment

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

max should be "2.x.x", as "3.x.x" and "4.x.x" are allowed to introduce breaking changes.

@@ -14,6 +14,9 @@
</author>
</authors>
<releases>
<release version="1.7.2" date="2017-05-19" min="2.5.x" max="4.x.x" php-min="5.6.x" php-max="7.x.x">
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get php-min and php-max from? As far as I can tell, it's not part of the schema, or at least it’s not documented? http://symphonyextensions.com/schemas/extension/1.0/#changelogs-and-compatibility

Copy link
Member

@nitriques nitriques Aug 22, 2017

Choose a reason for hiding this comment

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

That's why the PR is not merged ;) It's a change @alexnantel88 added in her 4.0 experiment. But it is soooo nice, that I'd like to add it for real. I'll check with her if she can redo the PR, but without any of the "experimental" stuff. Thanks for the review tho ;)

Copy link
Member

@jensscherbl jensscherbl Aug 22, 2017

Choose a reason for hiding this comment

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

Nice. Just wasn’t sure if it was an undocumented new feature in 2.7 already.

Copy link
Member

Choose a reason for hiding this comment

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

No no, I would not do that (voluntarily :P )

Copy link
Member

Choose a reason for hiding this comment

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

Strange, I thought that php-min and php-max was a new "de-facto standard". I vaguely remember that we had a discussion about it, but unfortunately I don't remember where and when.

Copy link
Member

Choose a reason for hiding this comment

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

@michael-e we agreed on this yes, but it was never implemented in an "official" way ;) I am still all for it !

Copy link
Member

@michael-e michael-e Aug 22, 2017

Choose a reason for hiding this comment

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

I am glad that you remember this — I was afraid that I only dreamed. :-)

Hmm, I already adopted it, e.g. here: michael-e/email_newsletter_manager@c396584

What should I do? Remove it? Or simply keep it?

Copy link
Member

Choose a reason for hiding this comment

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

It should not create a problem (for the computer) to keep it. But if you want to be strict, it should not be there.

Copy link
Member

Choose a reason for hiding this comment

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

:-(

Copy link
Author

Choose a reason for hiding this comment

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

@jensscherbl @nitriques
I've created a new branch (2.x) with the right release informations and removed the Symphony 4.x additions.

@nitriques nitriques changed the base branch from master to integration August 22, 2017 19:52
twiro and others added 3 commits May 30, 2018 15:31
PHP true,false,null in lowercase
SQL keywords uppercase
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.

5 participants