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

Function declaration typehinting for all classes and functions #711

Closed
alexweissman opened this issue May 20, 2017 · 3 comments
Closed

Function declaration typehinting for all classes and functions #711

alexweissman opened this issue May 20, 2017 · 3 comments
Assignees
Labels
Milestone

Comments

@alexweissman
Copy link
Member

PHP has function declaration typehinting for classes, arrays, and callables. Why aren't we using that consistently everywhere?

@alexweissman alexweissman added the standards and best practices Coding guidelines label May 20, 2017
@Silic0nS0ldier
Copy link
Member

Should definitely be used. It's an effective way to improve quality of code (in the worst cases) and can eliminate entire misguided debugging cycles.

Just keep in mind that string, int, float and bool are PHP 7 only.

@alexweissman alexweissman added this to the 4.1.0 milestone May 26, 2017
@alexweissman alexweissman self-assigned this May 30, 2017
@lcharette
Copy link
Member

Just keep in mind that string, int, float and bool are PHP 7 only.

I knew there was something special regarding this and PHP 7...

I guess it's something that can either be changed once, or as we go. If we want to drop 4.1 soon, it could be done as we go further, or at least moved to a 4.1.x version (shouldn't be any breaking change...)

@lcharette
Copy link
Member

I think I got most of it... I went thought all classes as pictured in 36affb0. Fixed the comments, classes and misc styling. Checked with Serenata in Atom and php-cs-fixer.

Two issues remains :

  1. Some non existing classes in WhoopsRenderer
  2. Serenata not being happy with this variable argument in ClassMapper

...but that's for another issue.

For now, we (and by we I mean @alexweissman) should probably setup StyleCI on this repo ❤️

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

No branches or pull requests

3 participants