11 ms·

The process is fine, but sometimes shit is broken and you need to fix it now and clean up the existing mess later. Why not instead of > Shirley (Code Review):

• absrnd 10y ago
  Because everything is urgent, all of the time.. and so nothing gets improved.
  • aiiane 10y ago
    This is what escalation is for - the job of a VP or Director of Engineering is to decide what is actually urgent and approve fast-tracking it, and what should actually follow process. Having an efficient escalation process is critical.
    • dsr_ 10y ago
      Very much this: there should be a process for "emergency change" and that process has to be used for every actual emergency and not for anything else.
    • droopyEyelids 10y ago
      The word "should"
    • chewyfruitloop 10y ago
      I've found that those in charge also have a budget to run you on... how do they justify you spending x hours updating code to do exactly 0 as far as anyone is concerned? ..plus all the regression testing? They just don't... it gets left as it is
  • ErnestedCode 10y ago
    "I know this is priority 1, but is it priority 1A, 1B, or 1C?"
    • quicklyfrozen 10y ago
      And let me guess... everything priority 2 or lower means 'never' :-)
    • NegativeLatency 10y ago
      Or priority zero.
    • ryandrake 10y ago
      Every numeric priority scoring system I've ever worked with at every company I've ever worked with has devolved into this: Start with P1 (high), P2 (medium), and P3 (low). Sooner or later, an exec will swoop in and declare something Even Higher Priority Than P1!!! So, "P0" gets invented, and after some time, you have tons of P0 issues, meaning P1 is the new "medium", P2 is the new "low" and P3 becomes "never". Then something comes in that is Even Higher Priority than P0!!! And now, you're stuck. For this reason, I always advocate reversing the numbers. The higher the P number, the higher the priority. So you never run out of numbers to express how much more urgent and earth shattering your next bug is. Try it!
      • zachrose 10y ago
        I just got a sudden bitter taste in my mouth when reading "P0" and remembering the people who used to say it out loud; is that normal? Another great thing about inverting the priority numbers is that nothing can easily go beneath a floor of 0, though ultimately all of this is about fixing the symptom and not the cause.
      • flukus 10y ago
        This is what I like about kanban, there is a queue, priority is not a status.
        • deleted 10y ago
  • user5994461 10y ago
    Nah. Nothing is ever urgent. Just take all tickets that are "urgent", remove the urgent and put them at the back of the queue.
  • kozak 10y ago
    The best solution against "priority inflation" that I ever saw: support request form of the DreamHost web hosting provider had priority levels with vivid descriptions (examples) of their meaning, to the extent that the top priority level was called "people are dying". I doubt anyone dared to select that priority unless people were indeed dying.
  • collyw 10y ago
    Story of my last few jobs.
• Spooky23 10y ago
  "The CTO mandated that we reformat the TPS reports by Friday. We'll add the change next week."
• 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.
• dragonwriter 10y ago
  > The process is fine, but sometimes shit is broken and you need to fix it now and clean up the existing mess later. Working in big enterprise shops, I've noticed that periodically that attitude does take hold, and the result is that the firefighting ends up producing more fires than it puts out. If you could change most of the people, culture, and incentives in a non-software-centered enterprise all at once (and this typically goes far beyond the IT shop), sure, maybe more fluidity in software development and release process would work better. But that's usually not possible, nor is it usually possible to durably wall off a group within an enterprise to isolate it from the problems of the larger organization (though that does happen sometimes.)
  • lostapathy 10y ago
    Exactly this. I think dev shops are fairly stratified into quality levels. "fix it now and clean it up later" works in a certain class of shop, and that's great. But there's a huge swath of dev shops that never pay any technical debt back, and the compound interest on that technical debt consumes all productivity. Past a certain point, drawing a line in the sand and forcing better practices no matter how easy it is to rationalize 'one more hack' is the only way to break that cycle.
    • jldugger 10y ago
      At least in this example, the entirety of the project involves fixing that "technical debt" of hard coding a variable, and it's being held up for not fixing _enough_ technical debt.
    • eitally 10y ago
      But for a lot of things, this is actually generally acceptable. Back office business apps, for example, where because of internal politics and attrition, they generally end up being replaced wholesale every 5-10 years, which is approximately how long it takes technical debt-ridden code to become so brittle it's unsupportable. A CIO can save millions by allowing this behavior. Thousands upon thousands of apathetic IT staff (including programmers) make decent livings in enterprises like this.
• ArkyBeagle 10y ago
  What is to keep Shirley from simply filing up to four new tickets? Isn't that better?
  • flukus 10y ago
    Shirley keeps finding problems everywhere, she's not a team player.
    • codycraven 10y ago
      Not only not a team player but Shirley is also a hypocrite. I guarantee there are non-standards based code touching the refractors she demanded at some level.
    • smallnamespace 10y ago
      Can you explain how finding problems is the opposite of being a team player? How is a team supposed to get things done if there isn't someone to recognize problems?
      • flukus 10y ago
        No I can't. It's just an attitude that I often see from management.
      • deleted 10y ago
        [deleted]
      • ArkyBeagle 10y ago
        Management is almost never incented to understand defects.
    • ArkyBeagle 10y ago
      Sigh
• Taek 10y ago
  It depends on the project. If this is critical code, 9/10 times it might not be an issue to let it slide in favor of getting the release out early, but that 10th time is a disaster. Weird things start happening and you don't know why and the team dealing with the fallout may not even realize which change caused the issue. So now there's a lot more disruption and a ton of frustrated people throwing way too much energy into finding what ultimately ends up being a one line mistake. After a few rounds of that you decide that if it's going to take 6 days, it's going to take 6 days.
• nashashmi 10y ago
  OT: I was about to post that women (Shirley) are bossy, but then I pretended that the named individual is Sam and the entire tone of the comment shifted immensely. It was as if the big boss was making this comment. And big bosses are never looking into things of this much detail, which gave more importance to the task. There is something that isn't right here. I am not sure why I am feeling this.
  • abritinthebay 10y ago
    Welcome to the scary world of recognizing your own biases! It's a good thing.
• droithomme 10y ago
  > The process is fine, but sometimes shit is broken and you need to fix it now and clean up the existing mess later. A problem is that most minor changes in the big picture are thought of as being critical things that need to bypass the process in the short term. It's not just this case that seems to make sense. It's that there's dozens or hundreds of other similar critical patches that need to bypass the process.
• st3v3r 10y ago
  Cause when you say, "You can fix it later," later never comes.
• bluejekyll 10y ago
  This is quite literally how technical debt is created. The story said that there was already a set of bugs being worked on. Which means that by adding this, the technical debt backlog would only continue to grow. I worked at a company where the technical debt was so deep that I really wanted to say that it was technically bankrupt, meaning that the software should be entirely rewritten. This goes against every grain of my experience to do, and we ended up not declaring technical bankruptcy, but if you want to understand where it starts, it's, we'll do it later. Each of those decisions needs to be accounted for just like a credit card purchase.
  • RijilV 10y ago
    I think that's the key - let's not forget what's at state with this story: laying off people. What's the price of extending the tech debt of this core code without 'proper' unit tests, hardcoded values, and employee-IDs in code? Is that greater than laying people off? It is easy to forget what the real world looks like when staring at a screen all day. We take out tech debt like a mortgage - sometimes that debt doesn't make sense and we end up having to foreclose on it, other times we're able to pay it off slowly over time and it all works out. Where the analogy to debt falls down is that sometimes you luck out and don't have to repay tech debt. Maybe your legacy manufacturing system gets replaced with something modern and all of that horrible code magically vanishes - maybe your single threaded software from the 90s is finally outdated and you need something modern and would have had to rewrite it anyhow.
    • badlucklottery 10y ago
      >Where the analogy to debt falls down is that sometimes you luck out and don't have to repay tech debt Just call that "winning the tech debt lottery". Much like winning the real lottery it definitely happens but not enough to rely on it.
• smallnamespace 10y ago
  It's not Shirley's job to make an executive decision, like whether to break policy or not. If the company policy should be ignored, then the issue should have been escalated to someone with the authority to make that decision. IMO the real problem is that the submitter didn't realize that was the case and so didn't escalate sooner. This could have all been avoided by asking for (and getting) carte blanche from Philip or David in the first place.
  • icebraining 10y ago
    Yeah, if I was Shirley, I'd just have suggested "why don't you ask David to overrule this one?", but definitively not overruled it myself.
    • jeroen 10y ago
      If I was Ed, I would have grabbed David and told him to tell Shirley to allow the change as is. Probably before submitting it for review.
  • Cthulhu_ 10y ago
    And yet, if everyone is aware not only of company policies, but the underlying principles, everyone also has the authority to deny something. Hardcoded values, for example, would violate a principle about maintainability. Talk about policy and it's politics and escalation and such. Talk about principles and it becomes a moral issue. Alternatively, think of it as breaking a specific law (such as the speed limit) vs being immoral in a more general sense of the word.
    • smallnamespace 10y ago
      I think making this a moral issue slightly misses the point. Even if Shirley understands the underlying principle of maintainability and is motivated to do the best thing for the company, she is still not the right decision-maker because the relevant risks and rewards are not fully visible to her. Decisions should be made at the lowest level where all the relevant information is available. In this case, the executive understands the pressing reasons for making the change, so is properly able to weigh the incremental risk (with the technical advice of Shirley et al) vs. the business priority, whereas Shirley can only see half of the picture. Imagine if Shirley had gone ahead and broken policy, then something went badly wrong. She likely would've been blamed for the whole fiasco.
• sundvor 10y ago
  This smells. I've seen plenty of the first part, fix it quick, but rarely any of the latter, fix it properly later. And the former stacks up rapidly as weeks turns to months turns to years.
• chewyfruitloop 10y ago
  >The process is fine, but sometimes shit is broken and you need to fix it now and clean up the existing mess later. when did you ever get to this land of milk and honey called later? whenever you have a jfdi because its broken, the way its coded always stays.... I've never seen a manager sign off on reworking code that does the job, just to make it "more manageable" or "standard" or "less of a bodge"
• Pxtl 10y ago
  > However, I'll allow the change for now because this is an important update that affects peoples jobs. I want to see every issue fixed before the end of the week and I will not approve any other changes until these issues are resolved. This goes in the backlog and gets pushed down further and furhter and further until it's backlog clutter that nobody even notices until that particular chunk of code no longer exists.
• Walkman 10y ago
  That's the worst thing you can do. The change will be forgotten and will never be properly worked out. Why should it? It just works. I have seen a 270k loc codebase where every change has been made by this kind of thinking. Nobody ever went back to make the "temporary" changes properly.
• raverbashing 10y ago
  Open a bug for the other changes and change only what's needed (unless they're literally in the same line and have no other secondary impacts)
• justinlaster 10y ago
  >The process is fine, but sometimes shit is broken and you need to fix it now and clean up the existing mess later. Because "later" might end up actually being in the middle of the night, or later might not ever come. And then the mess might be magnitudes bigger than what you were facing before. Just wait, now. You have the time. Always. Unless your platform/software is continually crashing, your business always has time. Changing code like that without review is begging for you to "not have time" and cause an undue amount of stress for everyone involved. Be an engineer, not a monkey. We're all only human and mistakes will be made.
  • sambe 10y ago
    I find this attitude on code reviews to be extremely demotivating, especially on code bases where I am not the primary committer or lack overall responsibility for the system being changed. It completely distracts from the code under review, and it only requires a small amount of social/organisational ill health to be abused. Even if you believe that the next committer should clean up, it can require quite a big context switch - either the clean up explodes into quite a big task and you have to question the trade-offs being made in doing it now, or it doesn't have full focus and becomes risky and/or no real improvement. I don't happen to believe that the next committer should clean up in most of these situations. Minor things, sure, you should do it (but don't side-track the review with it!). In my experience, people who already couldn't be bothered to do it or clean up later use a new commit as an excuse to get their work done for them. I've often seen people criticising tangential code issues in review who were responsible for the original commit. The question becomes - if it's such a minor issue for me to clean up, why wasn't it a minor issue for you to clean up? If it's bigger, how did it pass review in the first place? My view is that extraneous cleanup should be proportional to the original change at most - and not discussed in code review of unrelated changes. If a particular area of the code sees sufficient churn to accumulate more and more debt, everybody will know and talk about it, it will be clear that some specific refactoring is necessary, and more importantly it will be evidence that small cleanup is worthless (high churn) and a better architecture is valuable (high churn). A given team may have high tolerance for technical debt and thus never do this work. But then they are screwed anyway, why create noise at every review just hoping that someone else solves it? Remember, there's also a good chance that the mess stays the same as it always was, sitting there passively. And there's a chance that the cleanup introduces unanticipated problems, especially if it's legacy code with no/less testing.
• gaius 10y ago
  Right - the idea that a QA person would happily jeopardize people's livelihoods over a box-ticking exercise is why that profession gets a bad reputation...
  • deleted 10y ago
    [deleted]