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

allow paragonie/random_compat 2.0 #44

Merged
merged 1 commit into from
Apr 29, 2016
Merged

Conversation

ickbinhier
Copy link
Contributor

No description provided.

@xabbuh
Copy link
Member

xabbuh commented Mar 29, 2016

The composer.json file of the PHP 7 polyfill subpackage must be upated too.

@nicolas-grekas
Copy link
Member

Needs bumping to v2.0 of symfony/polyfill

@derrabus
Copy link
Member

@nicolas-grekas Why? As long as v1.0 is allowed, indicating compatibility with version 2 should not be a breaking change for Symfony Polyfills.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 15, 2016

For exactly the same reasons that made paragonie bump to v2.: apps falling-back to openssl would break.

@derrabus
Copy link
Member

Those apps should mark v2 of random-compat as conflicting and they're fine.

@stof
Copy link
Member

stof commented Apr 15, 2016

@derrabus but if they don't depend on it explicitly today (and they don't, as they get it through Symfony), they will still face a breaking change when updating deps, which is against semver.

@derrabus
Copy link
Member

If we continue this thought, Symfony with the same argument would not be able to indicate compatibility with version 2 of the polyfills until the 4.0 release, right? That means that we depend on random_compat v1 to be maintained and security-patched until November 2021, correct?

@derrabus
Copy link
Member

derrabus commented Apr 15, 2016

And actually, there's a precedent for this. Let's say, back in the days, I had created an app with these dependencies:

{
    "require": {
        "symfony/http-kernel": "~2.3"
    }
}

Composer installed the HttpKernel component along with HttpFoundation and EventDispatcher, all in version 2.3. If I updated this app now, I would receive HttpKernel 2.8 and HttpFoundation and EventDispatcher in version 3.0 including breaking changes. From my point of view, this is more or less the same issue.

And it's the same with Silex 1 applications. Those will be updated to Symfony 3 components as well, unless they actively block Symfony 3. And imho, this is absolutely fine, since Silex 1 remains compatible with Symfony 2.

To sum it up: Indicating support for random_compat 2 is (from my understanding of semver) fine for a 1.x release. Dropping support for random_compat 1 on the other hand would require a version bump. But this is not what this PR suggests.

@fabpot
Copy link
Member

fabpot commented Apr 29, 2016

Thank you @ickbinhier.

@fabpot fabpot merged commit 1f9d727 into symfony:master Apr 29, 2016
fabpot added a commit that referenced this pull request Apr 29, 2016
This PR was merged into the 1.1-dev branch.

Discussion
----------

allow paragonie/random_compat 2.0

Commits
-------

1f9d727 allow paragonie/random_compat 2.0
fabpot added a commit that referenced this pull request May 1, 2016
…well (derrabus)

This PR was merged into the 1.1-dev branch.

Discussion
----------

Allow paragonie/random_compat 2 in the php 7 subpackage as well

Greetings from the Symfony Live Cologne Hackday. :-)

This is a followup to the merged PR #44. I have added `"paragonie/random_compat": "~1.0|~2.0"` to the composer.json file of the `Php70` subpackage. Currently, only the full polyfill package allows version 2 of that library while the subpackage sill indicated compatibility with version 1 only.

Commits
-------

33b2cd3 Allow paragonie/random_compat 2 in the php 7 subpackage as well.
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.

6 participants