jsonrainbow / json-schema

PHP implementation of JSON schema. Fork of the http://jsonschemaphpv.sourceforge.net/ project
MIT License
3.54k stars 354 forks source link

supported version of PHP in 6.x.x #422

Closed mathroc closed 7 years ago

mathroc commented 7 years ago

in #390 I proposed to drop support for EOL version of PHP :

what about dropping support for EOL version of php ? by that I mean documenting that only php 5.6+ version are supported so that we can later use features from 5.6+ without having to release a new major version ? At first the lib would still run fine on 5.3

this could be done by marking travis test for those version with allow_failures so that support for those versions could still be visible and improved if wanted but wouldn't block new features

and here is @bighappyface answer:

@mathroc @erayd while I am open to dropping support for EOL versions of PHP we are obligated to supporting the same versions as composer being we are a dependency:

https://github.com/composer/composer/blob/eff9326b0de05551e976ca4ddcfc9f057fe29bcb/composer.json#L26

I would like to keep the discussion going but let's move that elsewhere.

so, this might be a valid point. but I'm not sure it's a problem for 6.x to drop support for version that are supported by composer. I can get a clear picture of the implications here as I'm not sure when composer update or composer install is run to build composer.phar

@Seldaek maye you can help us here ?

shmax commented 7 years ago

Moving my comment from the other issue:

while I am open to dropping support for EOL versions of PHP we are obligated to supporting the same versions as composer being we are a dependency:

I don't have a horse in this race either way, but is that really how this is supposed to work? Is there really an inverse dependency and obligation to be aware of other projects that are depending on this one? I think as long as we obey the rules of semver and bump our major release according to convention we can do what we like without danger of breaking anything, right? Composer seems to cap things off at the next minor branch of 5.0, and we're preparing a 6.0 branch, so I guess I don't see the problem?

mathroc commented 7 years ago

usually I would say that if your env is using an older version of php, you can continue using the older of the libraries as well (and maybe contribute patch to those older branches if you’re willing to).

for composer, it’a bit tricky, depending how it’s built, the issue I can see is if it’s built on a environment that can bundle dependencies that might not be compatible to environment where it’s going to be used.

so, supposing we release the next major without support for older version of PHP, composer have to wait until it also drop support from those version of php it seems to be using version 3, 4 or 5 atm so nothing is required but it won’t be able to use new features.

I don’t know if recent or future features of json-schema could be interesting for composer. If some are, it would seem fair - for the sake of helping such a useful project - to still support php 5.3

erayd commented 7 years ago

I think that as long as there is significant demand for PHP-5.3, we should continue to support that.

In principle I'd love to raise the minimum PHP version for this library, but not if it comes at the expense of a non-trivial number of users who need an older version. There isn't anything we need to raise the minimum version for at this stage - it's just a nice-to-have.

Does anyone know whether Composer wants support for anything newer than draft-04? If they do (and that seems likely), then that's definitely an argument in favour of retaining support for PHP 5.3, because they will need a newer version of this library in order to obtain that support.

bighappyface commented 7 years ago

^^ exactly

shmax commented 7 years ago

Looks like 5.3 is still a major player:

https://w3techs.com/technologies/details/pl-php/5/all

That said, if there were a nice, shiny PHP 7 version of this project I would switch to it (I'm kind of a hint-a-holic) in a second.

shmax commented 7 years ago

One option would be to have a few branches to support different major PHP versions, and do our backporting thing as needed.

erayd commented 7 years ago

@shmax

That said, if there were a nice, shiny PHP 7 version of this project I would switch to it (I'm kind of a hint-a-holic) in a second.

Is that a serious request?

I'm currently on my third complete rewrite (not yet on github) of that dogfood validator I was using for the info package development, and it just happens to be a nice shiny PHP 7 implementation... I never originally intended it for public release; it was just a way of dogfooding a package that was aimed at validator-writers - but if it would be useful, I can clean it up a bit, write some documentation, and put it on github / packagist.

erayd commented 7 years ago

One option would be to have a few branches to support different major PHP versions, and do our backporting thing as needed.

Are you volunteering to maintain that long-term? I'm happy doing the regular backporting, but I don't want to maintain another backports branch just for this.

shmax commented 7 years ago

Is that a serious request?

Well, as serious as I can be without making a big headache for everybody. The branching could get complicated, and I don't know what the implications would be for composer (I do know that you can target github branches in a composer file, but that might be imposing a bit harshly on github's hospitality for a project as popular as this one).

I'll confess I don't quite understand what your dogfood project is. Are you saying that you already have a 7'ified version of this package?

Are you volunteering to maintain that long-term? I'm happy doing the regular backporting, but I don't want to maintain another backports branch just for this.

Hmm, maybe I need to understand what the maintenance would be a little better before I get myself in trouble. I'd be willing to freshen all the code up and add all the hinting for an initial launch; what happens after that?

shmax commented 7 years ago

Is the idea that we would fork this project, bring it up to 7, and maintain it completely separately as its own animal?

erayd commented 7 years ago

I'll confess I don't quite understand what your dogfood project is. Are you saying that you already have a 7'ified version of this package?

No, it's a completely separate project. It's a complete strict-mode validator that passes the entire draft-03/draft-04 testsuite, including optional tests, and will shortly pass draft-06 as well. If that sounds useful to you, let me know, and I'll publish it properly.

The reason I wrote it in the first place is because I needed to usage-test the schema-info package, and so I wrote a complete validator implementation from scratch (three times) so that I could use the info package while doing so and expose problems with it.

Hmm, maybe I need to understand what the maintenance would be a little better before I get myself in trouble. I'd be willing to freshen all the code up and add all the hinting done for an initial launch; what happens after that?

You'd need to code-review every single commit and change the syntax / logic in such a way as to make it compatible with PHP-5.3, ideally without introducing new bugs, then merge the result to your backports branch. It's more complicated than the 5.x.x backports, because you will encounter many situations where the code will merge cleanly, but will not run without significant modification.

erayd commented 7 years ago

If the idea that we would fork this project, bring it up to 7, and maintain it completely separately as its own animal?

That seems like an awful lot of work for pretty minimal gain to be honest, and maintaining a fork just for that could easily confuse users. I don't think it's worth forking this; makes more sense IMO to just keep maintaining it here, in one place where everyone already knows it belongs.

mathroc commented 7 years ago

well given it easy to support php 5.3; there is not much to be gained yet from requiring php 5.6 (it’s mostly syntax sugar before 7/7.1) and that it can be useful for other projects to update json-schema while supporting older PHPs, I suggest we forget about that for the time being

fwiw, I don’t think having a fork or another version that’s php7 "ready" is that useful. or at least it makes thing much more complicated, increasing the risk of having bugs and increasing maintenance costs

shmax commented 7 years ago

fwiw, I don’t think having a fork or another version that’s php7 "ready" is that useful. or at least it makes thing much more complicated, increasing the risk of having bugs and increasing maintenance costs

Well, it can also make the code more readable and prevent bugs (with the additional hinting protection). But you're right that it would require more maintenance effort, in general. Maybe instead of thinking of 7 as the main branch and backporting to 5.3, we could do it the other way, where 5.3 is main, and there's a 7.x branch that is purely gravy and updated only by the whim of interested folks (such as myself).

erayd commented 7 years ago

...and there's a 7.x branch that is purely gravy and updated only by the whim of interested folks (such as myself).

Why? It adds more opportunity for bugs than it removes, and will mean bug reports need to be addressed in two places. In addition, if it's only updated as and when somebody is interested enough to do it, that will mean there could be a stagnant branch just hanging out there with historic problems. IMO better to do it properly, or not do it at all.

At the present time, creating and maintaining a PHP-7 fork or branch of this library feels like a fundamentally bad idea. Not saying it can't be done... but it feels like a complicated solution in search of a problem.

mathroc commented 7 years ago

Well, it can also make the code more readable and prevent bugs (with the additional hinting protection).

indeed (there will also be the weak type vs strict type debate :))

But you're right that it would require more maintenance effort, in general. Maybe instead of thinking of 7 as the main branch and backporting to 5.3, we could do it the other way, where 5.3 is main, and there's a 7.x branch that is purely gravy and updated only by the whim of interested folks (such as myself).

maybe a fork could work for that, the harder part would be to make the changes minimal to be able to rebase from upstream easily

mathroc commented 7 years ago

maybe a fork could work for that, the harder part would be to make the changes minimal to be able to rebase from upstream easily

also, will have to resist adding new features only in the fork because "that’s the one I use", "it’s easier" or "it’s faster"

shmax commented 7 years ago

IMO better to do it properly, or not do it at all

What is the proper way to do it?

erayd commented 7 years ago

What is the proper way to do it?

Off the top of my head, maintain strict behavior parity with upstream, provide sufficient resources to keep up with whatever upstream changes might be occurring, do not add branch-specific features. That requires a non-trivial amount of time, plus at least one other person to review your merge changes in that branch.

The main thing I was talking about with my 'properly or not at all' comment was the suggestion that a PHP 7 branch could be maintained on an 'as and when somebody is in the mood for it' basis. That isn't an appropriate maintenance attitude for a library as widely-used as this one.

erayd commented 7 years ago

With that said, there is nothing stopping you from maintaining your own private PHP 7 fork of this somewhere else for your own projects, if you care about PHP7-ising this library enough to do so. If you do this kind of thing in a way that only impacts you, you can do what you like :-).

shmax commented 7 years ago

Eh, no. If I was going to start fresh with a fork it would be to re-implement it for performance (probably by doing something similar to what AJV does, with generated functions), with PHP 7 as an afterthought. And there are other projects out there that I would target for a PHP 7 refresh long before I got around to this one (I've got my eye on https://github.com/jpfuentes2/php-activerecord, which is a dead project that a lot of people still depend on).

erayd commented 7 years ago

Fair call :-).

mathroc commented 7 years ago

alright, I’m going to close this then

thx everyone !

Seldaek commented 7 years ago

Just a quick note here as I was asked, from the Composer side if json-schema drops 5.3 support it's not that big a deal, we'd just not upgrade to the latest for a bit longer but we generally don't track the json-schema spec news very closely so it's ok. And a new version of Composer dropping <7 will be out sooner or later so at that point we can migrate. Anyway to sum up you don't need to hold back for us :)