Technical Debt vs. Precedence vs. Velocity
tomjose at linux.vnet.ibm.com
Tue Jan 23 20:35:45 AEDT 2018
On Tuesday 23 January 2018 06:58 AM, Brad Bishop wrote:
>> 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
I agree with Brad's suggestion that mixing code with different style is
fine. I have come across this debate a number of times in code reviews.
Developers would be put in a dilemma if they have to switch styles with
code in the current file. Its time to have a consensus on this aspect
at project level.
>> 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
>> 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?
More information about the openbmc