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

Update to PHP7 #83

Closed
wants to merge 2 commits into from
Closed

Update to PHP7 #83

wants to merge 2 commits into from

Conversation

oanhnn
Copy link

@oanhnn oanhnn commented Nov 26, 2018

I think we should update to PHP 7 (like psr/http-factory package):

  • PHP 5 is deprecated (PHP 5.6 Security Support Until 31 Dec 2018)
  • Using typed parameters and return value of PHP 7, interfaces is clearly visible.

@GrahamCampbell
Copy link
Contributor

This is a major breaking change. I think this would require a new PSR?

@webignition
Copy link

I'd say that a new PSR is not needed.

This is a breaking change for a minor release. If this was merged and then released as 1.0.2 or 1.1 (the current next logical release numbers), current PHP 5.6 users would have a very bad time. To merge and release as a minor version is quite clearly a bad choice.

This is not necessarily a breaking change for a major release.

We could merge and release as 2.0. The 1.x releases would exist for legacy (PHP < 7) users. The 2.x and onwards releases would exist for PHP >= 7 users.

Future changes would need to be applied to both the 1.x and 2.x releases. That's a little more work but given the relatively small size of this package I'd not consider the additional work to be overly burdensome.

Is there any release policy that prevents this approach?

@codemasher
Copy link

codemasher commented Mar 4, 2019

...which reminds me that i already did this master...codemasher:type-hints-php7.1
Might aswell submit a PR? v2.0 => PHP 7.0, v2.1 => PHP 7.1 (void & iterable)

@EvilFreelancer
Copy link

@codemasher hello, by psr-12 reasons can you please add one space after the colon followed by the type declaration? Thanks.

@codemasher
Copy link

There you go: master...codemasher:type-hints-php7.1

I've also changed the array type parameters of ServerRequestInterface::withCookieParams() and ServerRequestInterface::withUploadedFiles() to iterable to allow more flexibility.

@McLotos
Copy link

McLotos commented Apr 3, 2019

This is a major breaking change. I think this would require a new PSR?

Not needed. The whole serious developers use composer. Others may don't know about PSR.
PSR7 Must have a new branch (new tag for composer) for PHP7+ and save compatible with old PHP version in old branch.
At 2k19 not using a type hinting for arguments is bad practice.

@ghost
Copy link

ghost commented May 19, 2019

Have you considered files above ~2gb and integer overflow?

Methods like StreamInterface::tell() : int; and StreamInterface::seek(int $offset, int $whence = SEEK_SET); shouldn't be restricted to integer only.

@andig
Copy link

andig commented May 19, 2019

This would only be a problem on 32bit flatforms. Its int in php, too: https://www.php.net/manual/en/streamwrapper.stream-seek.php

@codemasher
Copy link

codemasher commented Aug 3, 2019

In addition to a PHP 7 upgrade, i'd like to propose to allow chaining of StreamInterface::seek() and StreamInterface::rewind() (codemasher@05c9c0b)

$message->getBody()->rewind()->getContents();

@jasny
Copy link

jasny commented Sep 2, 2019

@McLotos What's holding the merge of this PR / releasing it back?

@GrahamCampbell
Copy link
Contributor

@jasny Because this PR could be considered a change to the spec, and is certainly a major breaking change to the interface. There was discussion over if a new PSR is needed, or just a major version bump on the package. There are also implications for the virtual http-message package on packagist, which lots of people have dependence on * rather than ^1.0.

@jasny
Copy link

jasny commented Sep 2, 2019

@GrahamCampbell If the outcome of the discussion was that a new major version isn't an option, maybe close this (and other) PRs. At least, then it's clear they won't be merged.

@MarkusRodler
Copy link

If other people uses wrong dependency version strings (e.g. *), then that is their fault. SemanticVersioning FTW.
At least I am sure that they can fix their dependencies within seconds and I am sure that I will not stick to pre-7.0 standards that are outdated / state of the art. No interchangeability for me but I have type-safety.

This is only my opinion on this topic.

@GrahamCampbell
Copy link
Contributor

If other people uses wrong dependency version strings (e.g. *), then that is their fault. SemanticVersioning FTW.

Well, this was on a virtual package, which has no versions.

@McLotos
Copy link

McLotos commented Sep 3, 2019

I think, our discussion go to wrong way. What about this merge request and next public release?

@GrahamCampbell
Copy link
Contributor

I think, our discussion go to wrong way.

I don't agree. The PSR Amendments Bylaw doesn't allow changing the signature of interfaces, which blocks this PR: https://www.php-fig.org/bylaws/psr-amendments/.

@MarkusRodler
Copy link

Well then I see 3 possible ways out of this dilemma:

  1. Add a chapter in the PSR Amendments to allow versioning if newer PHP techniques are available
  2. Alternatively create additional PSRs for every "standard" that is out-of-date
  3. Or close all pull requests and stay with the non-strict-way

I don't know if it is possible to do the first one. The second one causes a huge process I think and the last one is no option in my opinion.

@Jean85
Copy link
Member

Jean85 commented Oct 4, 2019

We've been discussing this kind of issues a lot lately, and we've condensed our proposed solution to updating PSR interfaces in a blog post. Please read it and provide your feedback!

https://www.php-fig.org/blog/2019/10/upgrading-psr-interfaces/

@McLotos
Copy link

McLotos commented Mar 30, 2021

Hi @GrahamCampbell ! 2 years yet have passed, so when we can get a typed version? =)

@codemasher
Copy link

Hi @GrahamCampbell ! 2 years yet have passed, so when we can get a typed version? =)

An entire majpr PHP version has passed! Sorry, but at this point it seems more feasible to issue an RFC for PHP to allow adding method parameter type hints for poorly typed interface methods.

Copy link
Member

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

I'm now 👎 on the current form of this PR.

The bylaw for PSR evolution is now approved and in effect: https://www.php-fig.org/bylaws/psr-evolution/

It clearly states that we need a multi-step release, to avoid too many breaking changes. In particular, we need to add ONLY argument types first, and return types later with a major bump. We also need to require at least PHP ^7.2.

It has already been rolled out for:

With PSR-11 we got into some issues, but it's now resolved.

Ping @weierophinney who's the editor here.

@McLotos
Copy link

McLotos commented Mar 31, 2021

Hi @GrahamCampbell ! 2 years yet have passed, so when we can get a typed version? =)

An entire majpr PHP version has passed! Sorry, but at this point it seems more feasible to issue an RFC for PHP to allow adding method parameter type hints for poorly typed interface methods.

But in other packages it already done

@McLotos McLotos mentioned this pull request Jun 20, 2021
@oanhnn
Copy link
Author

oanhnn commented Feb 20, 2023

V2.0 support >= 7.0
V3.0 support >= 8.1 (Add Enums, readonly, ...)

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

@weierophinney
Copy link
Contributor

#94 and #95 do the appropriate steps that follow the PSR Evolution bylaw. As such, those will be what I use for the evolution vote, and I am closing this one.

BTW, @oanhnn : please read the bylaw. Adding enums, readonly support, etc. is out of scope as that would be substantive changes to the spec, and would require a replacement PSR.

@McLotos
Copy link

McLotos commented Feb 21, 2023

#94 and #95 do the appropriate steps that follow the PSR Evolution bylaw. As such, those will be what I use for the evolution vote, and I am closing this one.

BTW, @oanhnn : please read the bylaw. Adding enums, readonly support, etc. is out of scope as that would be substantive changes to the spec, and would require a replacement PSR.

current PSRs so outdated that it lost all reasons

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet