Skip to content
This repository was archived by the owner on Apr 23, 2023. It is now read-only.

Adds compatibility with PHP 7#35

Merged
fgrosse merged 1 commit intofgrosse:masterfrom
Spomky:master
Apr 28, 2015
Merged

Adds compatibility with PHP 7#35
fgrosse merged 1 commit intofgrosse:masterfrom
Spomky:master

Conversation

@Spomky
Copy link
Copy Markdown
Contributor

@Spomky Spomky commented Apr 21, 2015

  • FG\ASN1\Universal\Null renamed into FG\ASN1\Universal\NullObject
  • FG\ASN1\Universal\Null is now an empty class that extends FG\ASN1\Universal\NullObject
  • Tests configuration updated
  • Documentation updated

Fixes #34.
Please note that this commit may break compatibility and could require to bump to a major release

@Spomky
Copy link
Copy Markdown
Contributor Author

Spomky commented Apr 21, 2015

Could you please restart the test using PHP 7 on travis?
This test passed for me ; it seems it is due to a timeout and not to a bug frm this PR

@kgilden
Copy link
Copy Markdown
Contributor

kgilden commented Apr 21, 2015

For BC an empty Null class could be defined, which would extend the renamed NullObject. Which CS did You decide to follow btw?

Also, perhaps it would make more sense to split this into 2 - a CS PR and a fix for the reserved keyword. Would simplify reviewing a bit and keep things nice & tidy :)

@Spomky
Copy link
Copy Markdown
Contributor Author

Spomky commented Apr 22, 2015

PR updated

@kgilden
Copy link
Copy Markdown
Contributor

kgilden commented Apr 22, 2015

Looks good to me 👍

There's a couple of files under lib/X509 that still use the old Null object. Perhaps these should also use NullObject, but this can easily be done in another PR as well I guess.

@Spomky
Copy link
Copy Markdown
Contributor Author

Spomky commented Apr 23, 2015

@kgilden I have just updated my PR. I missed these files because no tests are performed.

I also added a @deprecated statement on class Null.

Adds compatibility with PHP 7
7be4ded 
* Tests configuration updated
* Null renamed into NullObject
* Null is now an empty class that extends NullObject

Please note that this commit breaks compatibility and requires to bump to
a major release
@fgrosse
Copy link
Copy Markdown
Owner

fgrosse commented Apr 28, 2015

Hey thx @Spomky and @kgilden for the PR and first review. I will have time today after work to do the merge (meaning this evening)

@fgrosse fgrosse merged commit 7be4ded into fgrosse:master Apr 28, 2015
fgrosse added a commit that referenced this pull request Apr 28, 2015
@fgrosse
Merge pull request #35 'Adds compatibility with PHP 7' into master
d03956e
@fgrosse
Copy link
Copy Markdown
Owner

fgrosse commented Apr 28, 2015

I decided not to bump a major version for this since you still left the old but now deprecated Null class lying there which I find a good solution for now.

Many thanks for the contribution :)

@Spomky Spomky mentioned this pull request Jul 4, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PHP 7

3 participants

@Spomky @kgilden @fgrosse