4 ms·

It seems like the code review policy should accommodate for scary legacy code. What happens if the findings are fixed later in the week? Are they promoted, to

by bmj 10y ago

It seems like the code review policy should accommodate for scary legacy code. What happens if the findings are fixed later in the week? Are they promoted, too? Are large swaths of the system regressed because of the changes? The process should allow the reviewer to say "oh, this is legacy code--the actual change looks good, and I won't worry about the code from ten years ago that doesn't conform to the standards we created a year ago."

0xffff2 10y ago
With that attitude all code is legacy code the day it is created, because it will never be updated to conform to future standards.
- bmj 10y ago
  That's a reasonable point, but how do you (as a product owner) justify the cost of making those fixes (and doing the regression testing necessary to ensure the system still works properly)? My group supports several legacy applications (10+ years old). The product owners can barely be bothered to green-light bug fixes. I can't imagine they could find the justification to green-light tech debt fixes.
  - couchand 10y ago
    I think the language of technical debt can actually be very meaningful to business. It's fair to take on debt in the short term to make initial gains, but you need to have a reasonable perspective about the ongoing cost that debt has, and pay it off on an ongoing basis if you can't afford the debt service. If it's a decade-old application that rarely even gets bug fixes, it's probably not worth doing major refactorings. On the other hand, if it's your core product, and your team is regularly making updates and fixes, it would be irresponsible to not incorporate your team's latest standards. Also, it's worth investing in test automation, because the balance really shifts if you can minimize the cost of regression testing by automating it.
mannykannot 10y ago
Fixing code to meet more recent standards has a nontrivial chance of introducing a bug. If it is to be done at all, it should be done on its own account, and not piggybacked onto something with its own goals. No matter how hard the process mavens try to reduce development to painting-by-numbers, good human judgement is the key to success.
- protomyth 10y ago
  Agreed, as that is the big issue. Anyone who requires a patch to production include more than the absolute minimum code change has a broken process. You make the next release bring that code up to the new standards when you can do a much fuller test.
- ScottBurson 10y ago
  Yes, totally agree. Far more risk is introduced by the cleanup than by the original change. The cleanup is still important, but shouldn't be fast-tracked.
- gsam 10y ago
  I get the intention here, but sometimes code really needs to be touched. Otherwise you get in the modern day, some-variant-of-Whitesmiths code that nobody can read, or wants to read ever. If you separate it out, business processes usually impede on your ability to ever get to the spun-off task and then it just gets forgotten.

by bmj 10y ago

0xffff2 10y ago

With that attitude all code is legacy code the day it is created, because it will never be updated to conform to future standards.

bmj 10y ago
That's a reasonable point, but how do you (as a product owner) justify the cost of making those fixes (and doing the regression testing necessary to ensure the system still works properly)? My group supports several legacy applications (10+ years old). The product owners can barely be bothered to green-light bug fixes. I can't imagine they could find the justification to green-light tech debt fixes.
- couchand 10y ago
  I think the language of technical debt can actually be very meaningful to business. It's fair to take on debt in the short term to make initial gains, but you need to have a reasonable perspective about the ongoing cost that debt has, and pay it off on an ongoing basis if you can't afford the debt service. If it's a decade-old application that rarely even gets bug fixes, it's probably not worth doing major refactorings. On the other hand, if it's your core product, and your team is regularly making updates and fixes, it would be irresponsible to not incorporate your team's latest standards. Also, it's worth investing in test automation, because the balance really shifts if you can minimize the cost of regression testing by automating it.

mannykannot 10y ago

Fixing code to meet more recent standards has a nontrivial chance of introducing a bug. If it is to be done at all, it should be done on its own account, and not piggybacked onto something with its own goals. No matter how hard the process mavens try to reduce development to painting-by-numbers, good human judgement is the key to success.

protomyth 10y ago
Agreed, as that is the big issue. Anyone who requires a patch to production include more than the absolute minimum code change has a broken process. You make the next release bring that code up to the new standards when you can do a much fuller test.
ScottBurson 10y ago
Yes, totally agree. Far more risk is introduced by the cleanup than by the original change. The cleanup is still important, but shouldn't be fast-tracked.
gsam 10y ago
I get the intention here, but sometimes code really needs to be touched. Otherwise you get in the modern day, some-variant-of-Whitesmiths code that nobody can read, or wants to read ever. If you separate it out, business processes usually impede on your ability to ever get to the spun-off task and then it just gets forgotten.