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

Resolve PHP code inspection warnings and errors in core /includes directory #1937

Closed
quicksketch opened this issue May 27, 2016 · 6 comments
Closed

Comments

@quicksketch
Copy link
Member

Similar to #1868, this issue is to resolve PHP inspection warnings and errors in the entire /core/includes directory. This includes fixing things like the following:

  • Missing docblocks.
  • Inconsistent return values or unspecified return values.
  • Removing unused variables.
  • Ensuring that @return values are accurate and interpreted correctly from callers.

This sort of checking reduces the errors reported by PHPStorm and other IDEs, allowing their inspection abilities to appropriately flag real problems in our code base. It also helps with autocompletion. And of course it should result in more complete and accurate documentation for real users reading the code or the docs on api.backdropcms.org.

@quicksketch
Copy link
Member Author

PR filed at backdrop/backdrop#1419.

@quicksketch
Copy link
Member Author

While fixing a PHP inspection warning in file.inc, I ran across a bug in creating new file entities. New issue at #1938.

@quicksketch
Copy link
Member Author

Merged in backdrop/backdrop#1419, it's only fixing a small (but critical) part of the documentation in Backdrop core, but it reduces the inspection warnings substantially. This will be in 1.4.3.

@klonos
Copy link
Member

klonos commented May 28, 2016

👏 for reducing annoyances.

@quicksketch
Copy link
Member Author

I found that PHP 7's error handling doesn't work quite right with one of these changes. We made it so that _backdrop_decode_exception() took a type hint of Exception, but in PHP 7, this may be either an Exception or Error object. PR filed to fix this at backdrop/backdrop#1431.

@quicksketch
Copy link
Member Author

Merged backdrop/backdrop#1431. Again for 1.4.3.

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

No branches or pull requests

3 participants