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

Please support older versions of PHP #6

Open
bvisness opened this issue Oct 23, 2019 · 10 comments
Open

Please support older versions of PHP #6

bvisness opened this issue Oct 23, 2019 · 10 comments
Labels
question Further information is requested

Comments

@bvisness
Copy link

My company is currently using the jimdo version of this SDK and found out today that the package has been archived and you have taken over maintenance. We still have projects on 7.1, but it looks like you recently changed this library to only support PHP 7.3 and up.

Since jimdo was the best-supported PHP SDK for Prometheus (and the one linked from the official site), and since jimdo supported PHP 5.6 and up, changing the minimum version like this is pretty frustrating and is preventing us from using this library.

Now that you are maintaining this library for more than just your employer, please consider making it your policy to support all versions of PHP that are still officially supported. As of now, this would mean supporting PHP 7.1 and up. Anything else will prevent otherwise law-abiding people from using this library.

@NoelDavies
Copy link

Hi @bvisness, newer versions of PHP are implemented from v1.0.0 of the package, and as-per semver, is marked as a breaking change due to method signatures.

PRs will still be accepted for security patches and small features for older versions of PHP (and those PRs will be released with <1.0.0 tags.)

Please ensure you're not affected by the breaking version change by appropriately using constraints in your dependencies https://getcomposer.org/doc/articles/versions.md#stability-constraints.

@NoelDavies NoelDavies added the question Further information is requested label Oct 24, 2019
@bvisness
Copy link
Author

bvisness commented Oct 24, 2019

Since PHP 5 is completely end-of-life, I think it makes sense to release a new major version requiring PHP 7. But locking the version to 7.3 and up is completely gratuitous. (And furthermore, will require another major version to be released, since you seem to already have released 1.0 with a dependency on 7.1.)

As a library author you are shooting yourself in the foot by requiring people to use the absolute latest version of PHP. I like type hints as much as the next guy, but the reality of the situation is that not everyone can update right away. Nor should they, if their version is still officially supported.

Are a couple fancy language features worth losing a large chunk of users? If so, I wouldn’t be surprised if my company ends up making a fork just to remove the type hints and make it compatible with 7.1 again.

@NoelDavies
Copy link

NoelDavies commented Oct 24, 2019

@bvisness I completely understand your issue, and we should have perhaps done a slow roll-out as older versions were no longer supported. Nothing stops users of the repository locking themselves to versions <1.0.0.

(Sidenote: the 7.1 release was accidental, and it should have actually been tagged 7.3.)

I'm more than happy to work on a pre 1.0.0 release with you to add some upgrades without forcing the use of PHP 7.3.

As the now official fork, we'd like to support all users where possible, whilst maintaining the ability to adding new features, reducing technical debt and keeping security at the front of our approach to development.

If you'd like to submit a PR for these changes, we can merge that and release that with a few other changes, as a release under 0.10.0.

@piotrooo
Copy link

@bvisness first question - who will be maintaining that old code? Who will be making backports to the old version for unsupported PHP version? When you contribute code to the 7.3 version there are BC changes, that can't be just merged, needs a manual intervention (e.g.: method signatures).

IMO if someone can't be upgraded to 7.3 they should use a <1.0.0 - as a @NoelDavies said.

Also, I think there should be created branch 1.0.x from which will be created a new tags for version 1.0.1 ... 1.0.n. (trunk-based development).
How this should works? https://paulhammant.com/2013/04/05/what-is-trunk-based-development/
That's only an idea, so we can disscus about it.

@bvisness
Copy link
Author

My suggestion was not to maintain patches for unsupported versions of PHP. My suggestion was for this library to support all supported versions of PHP instead of just the latest.

You say “there will be BC breaks when contributing to the 7.3 version”, but my point is that there should not be a “7.3 version”. There should be a 7.1+ version, until 7.1 is no longer supported, at which point you can move to a 7.2+ version.

You only get BC breaks if you use the absolute latest and greatest features, which in this case are not actually necessary for the actual function of the library.

Basically I think you have a tradeoff between two options:

  • Stick with the latest. Developers who can use the latest version will get some extra type hints, but many people will be completely unable to use the library.
  • Support all supported versions of PHP. Users of the library may not have as much type support, but everyone will be able to use it.

As the pretty-much-official Prometheus library for PHP, I think the second choice is clearly the correct one.

@NoelDavies
Copy link

NoelDavies commented Nov 19, 2019

Due to PHP 7.1 being FULLY EOL in 11 days (Dec 1 2019) we will not be adding PHP 7.1 support. But I think we can add PHP 7.2 support, but this needs to be a community effort.

@piotrooo
Copy link

@NoelDavies agree with that. But firstly we must think how to organize code. Master should be 7.2 compatible? We need another branch. This is most important decision - IMHO.

@bvisness
Copy link
Author

I think master should be 7.2 compatible. The differences between 7.2 and 7.3 should be minor enough that it will run fine on both (especially for a library that previously ran fine on 5.6), and maintaining two separate branches would be a ton of work.

@NoelDavies
Copy link

Agreed, We need a 7.2 addition to the CI pipeline, and go from there. Does anybody wanna start this? I'd be happy to contribute too.

@bvisness
Copy link
Author

bvisness commented Jan 6, 2020

I created #35 to add PHP 7.2 to the CI config, and it seems to be "working".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants