Technical Debt vs. Precedence vs. Velocity

[Brad Bishop]

> On Jan 19, 2018, at 11:30 AM, Vernon Mauery <vernon.mauery at linux.intel.com> wrote:
> 
> Overall, the openbmc project has done a great job of keeping common things in common repositories and machine or OEM specific things in separate repositories. But there are a few (because it happens) places where technical debt has accrued. I want to support getting this sort of stuff fixed so we can have a shiny project, but at what expense? Velocity. Reality calls and I was not scoped to fix old code, only to write a new feature. I am not the only one running into this issue, so I thought it best to start a discussion.
> 
> Things like:
> 1) Incorrect coding style (whitespace, braces, upper/lower/snake/camel)
> 2) Use of sdbus instead of sdbusplus
> 3) OEM code in common libraries
> 4) Lack of proper error handling
> 5) Use of insecure functions without proper range checking (memcpy, strcpy, etc.)
> 6) Use of printf/fprintf instead of log<>
> 

Use (your maintainer) discretion, but I think precedence would _rarely_ be
a valid justification for new technical debt.

Our coding style, frameworks, guidelines, etc are all going to change over
time.  The codebase will never be up to date in terms of the current mode
of thinking.  So I would advocate for mixing code that does meet the current
guidelines with code that does not, otherwise it will just get progressively
worse.

> I am sure there are many other classes of technical debt that I did not list here. But the point is that the code base does have places where this sort of stuff exists and has not yet been fixed. What should the official stance be? To me, mixing coding styles in a single file is worse than having the same wrong coding style in a file. But the best is if the whole project matches.
> 
> If someone contributes and claims that their newly added technical debt is okay

Maybe just write down the rules you want, and put them in a README in the
project repos you maintain.  Then you can simply point at this when these
come up in review.  Just make sure your README files get a good review
as well.

I suspect some will want things like this documented at an OpenBMC wide level
and not a per repository basis.  I’m not sure if I like that or not.  Maintaining
is hard, and we all have our different things that make it less hard for us to
maintain.

> because of precedence, we don't really have a way to hold them to coming back and fixing the problem.
> 
> One way to fix this is to have contributors that want to claim precedence go and clean up the file FIRST and then come back with a new clean commit. But that does make for a higher bar for entry in some cases and possibly slows down velocity.

I think this is OK as long as the patch submitter has another option - adding
code that meets the current guidelines without requiring the fix-up.  If they
have a personal issue with mixed styles, they can make the fix-up prior to
their edits.

> 
> What say the community members on this?
> 
> --Vernon