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

More Phan Fixes #3535

Closed
wants to merge 10 commits into from
Closed

More Phan Fixes #3535

wants to merge 10 commits into from

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Mar 5, 2018

This does another round of randomly fixes some of the smaller, low hanging fruit errors detected by phan.

@@ -619,7 +619,7 @@ class Database
*/
function selectRow($query, &$result)
{
$this->select($query, $result);
$result = $this->pselect($query, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

[] => array()
Unless you want to start a new convention

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:(

@@ -641,8 +641,7 @@ class Database
*/
function selectOne($query)
{
$result = null;
$this->select($query, $result);
$result = $this->pselect($query);
Copy link
Contributor

Choose a reason for hiding this comment

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

this require a second argument

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I guess that's one of the error types which are still suppressed by phan.. I'll fix it.

@xlecours
Copy link
Contributor

xlecours commented Mar 6, 2018

The Travis is failing because of phpcs in htdocs/survey.php
42 | ERROR | Missing short description in doc comment

@driusan driusan dismissed xlecours’s stale review March 6, 2018 17:38

updated with changes (..but review wasn't autodismissed..)

@@ -199,7 +199,7 @@ function handleOPTIONS()
{
$this->Header(
"Access-Control-Allow-Methods: ".
join($this->AllowedMethods, ",")
join(",", $this->AllowedMethods)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make more sense to use implode() in lieu of join() as PHP docs description is in implode() and join() page refer to implode() page.
Not a blocker, just a discussion

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but this is the minimum change to fix the "PhanParamSuspiciousOrder" error

@driusan driusan mentioned this pull request Mar 12, 2018
@driusan
Copy link
Collaborator Author

driusan commented Mar 19, 2018

This was included in #3544, but not auto-closed because the merge was squashed..

@driusan driusan closed this Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants