Technical Debt vs. Precedence vs. Velocity

[Deepak Kodihalli]

On 19/01/18 10:00 pm, Vernon Mauery 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<>
> 
> 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 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.
> 
> What say the community members on this?
> 
> --Vernon
> 

I suppose this problem is more pronounced with the ipmi repos, and I 
think the reason is most of that code was written much before the 
current set of coding practices and guidelines have been established.

While in some instances I've posted comments asking the author to rework 
their commits based on the current standards, it's most of the times not 
feasible because of the mixing coding guidelines problem you brought 
out, which I agree is odd to do. At the same time, I'm not sure how apt 
it is to ask a contributor to clean up the file in question first, if 
that's even possible to do at a file level, when their commit is 
intended to solve a specific problem and it does that job.

Sorry I don't have any answers for this right away, the one thing I can 
think of is planned effort to refactor the ipmi code base to meet our 
standards. I kind of feel that might lead to better results than 
cleaning up/refactoring on the go.

Thanks,
Deepak