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

Resource will be a reserved keyword #119

Closed
EmanueleMinotto opened this issue Sep 21, 2015 · 14 comments
Closed

Resource will be a reserved keyword #119

EmanueleMinotto opened this issue Sep 21, 2015 · 14 comments
Labels
Bug
Milestone

Comments

@EmanueleMinotto
Copy link

@EmanueleMinotto EmanueleMinotto commented Sep 21, 2015

Quoting Style CI:

In php 7, "resource" is a reserved name and you can't use it for a class

Ref: https://twitter.com/TeamStyleCI/status/645906122524753920 & https://wiki.php.net/rfc/reserve_even_more_types_in_php_7

This could be a problem for Puli\Repository\Api\Resource\Resource, should it be renamed?

@tgalopin
Copy link
Contributor

@tgalopin tgalopin commented Sep 21, 2015

It does not seem to be a problem: https://travis-ci.org/puli/repository/jobs/80213636. Is it something not yet present in the PHP7 nightly of Travis?

@EmanueleMinotto
Copy link
Author

@EmanueleMinotto EmanueleMinotto commented Sep 21, 2015

I didn't investigate yet. Could it be a big BC?
Anyway this problem could be ignored until the release of puli (imo).

@tgalopin
Copy link
Contributor

@tgalopin tgalopin commented Sep 23, 2015

It is definitely a problem for StyleCI, but we can disable the phpdoc_types fixer to ignore it. Would it be fine for you @webmozart ?

@webmozart
Copy link
Member

@webmozart webmozart commented Sep 23, 2015

Yes, let's do that for now.

@stof
Copy link
Contributor

@stof stof commented Sep 29, 2015

resource does not trigger an error in PHP 7, but they document that it should be considered reserved for future use: http://php.net/manual/en/migration70.incompatible.php#migration70.incompatible.other.classes
This is why StyleCI is complaining.

Given that renaming it in Puli is a BC break, it would be better to do it before reaching 1.0 instead of facing the need for a BC break later on if PHP actually makes use of it as a reserved keyword.

@webmozart
Copy link
Member

@webmozart webmozart commented Sep 30, 2015

Any suggestion for a good name? PuliResource?

@EmanueleMinotto
Copy link
Author

@EmanueleMinotto EmanueleMinotto commented Sep 30, 2015

PuliResource 👍

@webmozart webmozart added this to the 1.0-beta10 milestone Oct 7, 2015
@stof
Copy link
Contributor

@stof stof commented Oct 7, 2015

@webmozart I can't think of a better suggestion

@webmozart
Copy link
Member

@webmozart webmozart commented Oct 7, 2015

Well we still have PResource, Resourze or Resource_ 😎

@webmozart
Copy link
Member

@webmozart webmozart commented Oct 7, 2015

On a serious side, let's go for PuliResource. Anybody wants to work on that? (@EmanueleMinotto?) We also need to enable the phpdoc_types check again.

@EmanueleMinotto
Copy link
Author

@EmanueleMinotto EmanueleMinotto commented Oct 7, 2015

@webmozart sure, I can do it 👍

@webmozart webmozart added the Bug label Oct 8, 2015
@webmozart
Copy link
Member

@webmozart webmozart commented Oct 8, 2015

@EmanueleMinotto Will you have time to fix the other repositories soon? Currently, most repositories are broken on master.

@webmozart
Copy link
Member

@webmozart webmozart commented Oct 9, 2015

This is finished now. :)

@webmozart webmozart closed this Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants