[Discuss] php dev's code with warnings and notices

[richb at pioneer.ci.net (Rich Braun)]

Eric Chadbourne asked:
> The code they have submitted works,
> but it has a bunch of warnings and notices in the logs.  I personally
> think this is sloppy coding.  My question is, how strong a stand should
> I take on this issue?  I have the senior role but I am also the "new
> guy".

I've recruited a team of Ruby coders over the past year, and despite the
"green-fields" status of the project (brand-new code-base) we found over 6000
exceptions in the code the first time we ran a tool called Rubocop to look for
syntax sloppiness.

We got one intern for the summer, a strong-willed Stanford sophomore, so I put
him on the task of code cleanup.  A couple days later, he pushed back with
this very coherent argument: if you have me touch 150+ files but don't have
full unit-test coverage, aren't you asking for more trouble than this cleanup
is worth?

My suggested response was that we gate the build pipeline with a Rubocop check
(along with other static-analysis tools) for all newly-modified files,
ignoring problems with the older files.

Ultimately, we wound up just in the past few days opting to accept all of the
intern's cleanup; we'll fix whatever logic problems crop up.  But the team's
still not fully convinced we've made the right choices.

My suggestion for you, Eric, is to ignore the problem unless you have
management buy-in (and a reasonable level of team consensus) to do the
following:

* Implement a unit-test code coverage tool that reports all lines that aren't
covered by unit tests, if you don't have that in place already, and run it at
least daily if not every build;
* Get the test coverage well above 50%;
* Block all code check-ins that worsen the problem (i.e. implement a
static-analysis gate).

Gating code check-in deserves its own post; there are git hooks, Jenkins
static analysis scripts, etc; team members always have strong opinions about
when to run what tools against which files during the development process. 
And no two people seem to agree on the integrated development environment
(IDE) choice.

-rich