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

Scrutinizer error tracking issue #4176

Closed
MrPetovan opened this issue Jan 5, 2018 · 5 comments
Closed

Scrutinizer error tracking issue #4176

MrPetovan opened this issue Jan 5, 2018 · 5 comments
Assignees

Comments

@MrPetovan
Copy link
Collaborator

MrPetovan commented Jan 5, 2018

https://scrutinizer-ci.com/g/MrPetovan/friendica/issues/task/4176-scrutinizer

Log:

  • January 16th 2018: 1691 issues
  • January 19th 2018: 1681 issues
  • February 7th 2018: 2018 issues (!?)
  • February 13th 2018: 1964 issues (thanks @rabuzarus)
  • February 14th 2018: 1898 issues
  • October 22th 2018: 2222 issues (!!)
  • December 25th 2018: 2459 issues (!!!) but the code quality score jumped from 2.9 to 3.54 out of 10!
  • December 26th 2018: 2219 issues ([Scrutinizer] Replace deprecated method/function calls #6327)
  • January 7th 2019: 2344 issues (because of the deprecation of Logger::log)
@MrPetovan
Copy link
Collaborator Author

@MrPetovan added Tedious Necessary labels a day ago

@annando
Copy link
Collaborator

annando commented Jan 5, 2018

In German you would say "Mühsam ernährt sich das Einhörnchen" (comparable to "Little by little, the bird builds its nest.")

@MrPetovan
Copy link
Collaborator Author

Here's how static analysis can concretely help a project code health in unexpected ways: When I updated dba::select() return type documentation to reflect the possibility of an array in case of limit => 1, the next Scrutinizer inspections showed a bunch of issues regarding type handling around the fact that we were calling array-only functions on the return data of dba::select(limit => 1).

It then hit me that we were doing two very different things with dba::select() and that it should probably be split into two distinct functions. This helped:

  • Create dba::selectFirst() with a clear and simple purpose and non-ambiguous return types.
  • Make dba::select() clearer and simpler with less conditions and non-ambiguous return types. I removed an input parameter ($params[only_query]) and a variable ($single_row) that didn't need to be moved to the new function.
  • Simplify all the dba::select(limit => 1) calls to dba::selectFirst().

I probably could have come up with the change myself but the fact is that I didn't, and only my commitment to fix issues brought up by Scrutinizer finally prompted me to do it.

@tobiasd tobiasd added this to the 2018.11 milestone Aug 1, 2018
@MrPetovan
Copy link
Collaborator Author

I finally got to run Scrutinizer again after the TLS debacle surrounding git.friendi.ca, and unfortunately we jumped 300 issues since last February.

BUT the Scrutinizer Code Quality Score jumped from 2.91/10 to 3.47/10, so we must be doing something right.

@MrPetovan
Copy link
Collaborator Author

Currently the output of Scrutinizer isn’t useful, too many false positives, closing here.

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