14 ms·

6 days to change 1 line of code (2015)

358 points by tempodox 3y ago

shpx 3y ago
Better than waiting a year or more for an open source project to make a release bumping a dependency because no one cares.
- tbarbugli 3y ago
  I disagree, you can fork and apply whatever change is blocking you to the OSS project.
jacknews 3y ago
Or in other words, artificial/arbitrary management metrics/deadlines force through a possibly dangerous change to factory operations?
- yosefk 3y ago
  What procedure mitigating the supposed risk of this change was skipped due to management pressure according to TFA? The way I read it, the original change of the constant "3" to "4" was tested, it was observed to have the desired effect, and none of the delays before the coding and the testing by the developer or after it did anything to reduce the risk further.
  - jacknews 3y ago
    That would be the (non-existent) tests that should perhaps be run because "This affects everything in the factory." Probably the code still works with this change, but does the factory?
- raverbashing 3y ago
  Yes Because, in this case, the buck stops with him All deadlines are "artificial" and some have a lot of $$$ attached to them (due to regulatory/market/customer/supplier issues) But I know, a blame culture begets a CYA culture, which begets the opposite of "help me help you" (hence why such a parameter is hardcoded in the code - the issue here is not stopping the ~~orphan~~ developer crushing machine but why does it exist in the first place)
  - deleted 3y ago
    [deleted]
  - jacknews 3y ago
    A CEO is entirely responsible for the culture of an organization.
crop_rotation 3y ago
That is absolutely true. Most companies have code review process that is totally full of nitpicking and trivial comments. I once suggested replacing that with static analysis tools to remove such comments and make feedback faster in our org, and was told that such code review is necessary for everyone. It helps people get promoted, makes people feel they prevented code issues, keeps the code review metrics looking good for the top level overlords who sit and monitor number of reviewer comments.
- javajosh 3y ago
  It depends on your team. If you have good juniors, they will relish the feedback. Of course it needs to be substantive. Often junior code will be far more complex and difficult to read than senior code, and code reviews are a chance to get that feedback across. That said, often teams don't give credit for code reviews, or take into account the (sometimes considerable) time required to write them. To some extent, tools are also to blame - I think all teams would do well to pick a system that integrates with the dominant IDE. Doing code reviews in the same space you write code just makes sense; requiring that you use a friggin' webapp is HORRIBLE.
  - crop_rotation 3y ago
    The thing is, almost all the trivial comments can be easily automated. That would both give the junior dev quicker feedback before even raising the review, and save time for everyone.
    - javajosh 3y ago
      >Of course it needs to be substantive
    - morelisp 3y ago
      Junior code is not usually more complex because of bad naming conventions and inconsistent styling and related issues that tools can help with, although of course these don't help its readability overall. Rather it usually has control flow that's far too literal a translation of requirements leading to an exponential explosion of paths, insufficient and/or over-eager error handling (often both at the same time), poor representational choices leading to inefficiency or more special cases, and so on. None of these can be effectively automatically detected.
iLoveOncall 3y ago
Not sure I see the point, rules are in place for a reason and it's not like changing 2 lines would have taken 12 days. This seems like it changes something with potentially super-high impact, 6 days from request by the CEO to release to production is a pretty good release time.
- crop_rotation 3y ago
  It is not a good release time for the described scenario. The company badly needed a change to survive so fixing unrelated issues in the same change doesn't make sense.
  - iLoveOncall 3y ago
    I could somewhat agree with that, however it seems like this only added a couple of hours out of the 6 days.
  - icegreentea2 3y ago
    The problem (and solution) isn't necessarily to change the process. Assuming good faith and competence when designing the process, all (or similar) of the downsides encountered in the scenario were considered when designing the process, and the cost-benefit came out ahead to add the extra safeguards the layers. As demonstrated by the end of the scenario, the 'solution' was the president to more directly intervene. The failure in this scenario isn't necessarily that the process got in the way, it was that when the President and IT Director kicked off the action, they were not fully aware of the efforts and overrides required to escalate to 'do this now'. A converse failure is that the intent of the President and IT Director was not properly communicated to all stakeholders in the process.
  - moralestapia 3y ago
    >The company badly needed a change to survive [...] Lol, nah. This is fiction, no company IRL is going to start laying off people after a few days of noticing a small dip in production. The whole premise of "We own this company but if this code change doesn't go through in X days we will fire people instead" is completely stupid. But it is an entertaining story and it gets the point through that internal bureaucracy gets in the way of many things.
don-code 3y ago
There's another failure mode not described here: "product management doesn't understand the change". I'm in the (perhaps enviable, perhaps unenviable) position of getting a small subset of my marching orders directly from our CTO. Engineering at my company is organized in such a way that, while engineers report to the CTO, product management (part of engineering) does not. When the CTO asks me to priority-override a new feature, or even a small change, I've sometimes gotten into arguments with product management over how this wasn't planned for, and how they need time to adjust schedules and collect requirements before I can begin work. Unfortunately, I can't use the "talk to your boss" excuse, due to that quirk of the reporting structure. One memorable example of this was a feature request which came in hot in mid-June, which I had code complete later that day. We actually shipped it a few days ago. To product management's credit, they _did_ come up with requirements that were not met by my initial change, but I question whether releasing fast and iterating, or releasing correct after four weeks, was better for the customer.
- throwaway2990 3y ago
  Sounds like a horrible place to work.
- icegreentea2 3y ago
  Why can't you use the talk to your boss excuse? Does your CTO (and whoever PMs ultimately report to) intend for these types of friction to occur every single time a CTO request appears? Does your CTO notice that these small changes takes extra time to occur? Does your CTO even care?
  - jstanley 3y ago
    Because the CTO is not the product manager's boss.
    - icegreentea2 3y ago
      My point is that the product manager also has a boss, and both the CTO and that boss have a shared boss (the CEO). If the CTO and product boss are constantly stepping on each other's toes (or whatever), that's a problem that they either need to solve, just acknowledge as just the cost of doing business. And that ought to be communicated and acknowledged by everyone in the line.
politelemon 3y ago
> Julie: Then contact Joe in IT Security. He'll get you permissions. > 2 hours later. This is completely unrealistic, security teams would never respond so quickly.
- seanthemon 3y ago
  2 programmer hours, obviously
- 542458 3y ago
  Hah, I have a very different experience where I work. At my org every time I submit a ticket requesting access for somebody to such-and-such a system it typically gets done within a couple minutes regardless of what priority I give. I’ve sometimes wondered if help desk staff snap them up as soon as they come in because they’re fast tickets to close and lets them improve their personal metrics.
- switch007 3y ago
  Unless it’s them getting a P1 security alert because you ran “npm install”
- fargle 3y ago
  our security team actually responds even faster. they simply deny all requests automatically, but immediately.
- aezart 3y ago
  It takes multiple weeks to get someone added to the AD group required for wiki edit permissions.
short_throw 3y ago
I was just waiting to hear that the parameter loading wasn't available in some edge case scenario and code review changes took down the system. But actually this is a story about the process working.
zac23or 3y ago
A few years ago, working on a comment team, I created a function to do an AB test using an AB test team system. Someone else does the implantation, not me. Someone didn't like the function, as they didn't use the company's cache system (the AB team previously explained all the reasons why the cache is not used, etc). I started to participate in several meetings to explain that it does not use the cache system because bla bla bla. A week or two after a few meetings (and a lot of money spent), I got it deployed. It's the same company that created a new meeting to discuss wasted time in all the other meetings.... At the time I thought: This is abnormal, but it is actually an extremely common scenario in many companies.
- crop_rotation 3y ago
  You might be surprised at how common this is.
- jawns 3y ago
  > It's the same company that created a new meeting to discuss wasted time in all the other meetings.... I've been in this kind of meeting. It was a 30-minute meeting instituting a rule that meetings should be 25 minutes by default.
  - scruple 3y ago
    I sat through that one, too.
- duxup 3y ago
  I worked for a company with a few thousand employees. Everyone had to spend about 6 hours in meetings about how to have more effective meetings. VPs and managers droned on about how important it was. Rule 1 in the training was to have a detailed agenda. As far as I saw nobody ever followed rule 1 except myself and one other person. I’m glad I work at a small company now.
swader999 3y ago
This is a O(1) operation at this company. Did I pass the interview? I still don't want to work there.
zerr 3y ago
Anecdotal experience: after some years in the team with formal code reviews, moved to the team/company without code reviews. Everyone is free to commit/merge to any branch. When joining the company, I had somewhat mixed feelings about this. Apparently, it felt so refreshing and empowering, I became productive within few days.
- jefozabuss 3y ago
  sounds like a security and audit nightmare to be honest but I guess it’s an agency or similar with smaller projects
- anthomtb 3y ago
  A while back, I also worked for a team that did "Catholic Code Reviews" (push and pray). No code review worked very well given the objectives of the team. It was an R&D group who's primary objective was "demo the nifty new thing to the executives". That meant a lot of short notice requests but also a lot of throwaway code. We'd demo the thing, the exec would be like "looks great, no business case" and the repo would never be touched again. Of course, every now and then our thing WOULD get productized and the downstream team would be responsible for turning chicken-scratch code into something production worthy. Those guys hated us with a burning passion.
- swader999 3y ago
  I've seen this work really well on small teams where there's high degree of trust, ~80% test coverage. It was a no PR process, just merge with master when tests pass, UX(if applicable) demoed successfully to stakeholders and you are satisfied with it. New team members were assigned a mentor that sat beside and frequently paired and reviewed their code for the first 2-3 months. Was a 2.5 year project, went live at 20 months in, was on time and meet budget with more capability than originally scoped. Many days had 2-3 hours of discussion at the whiteboard. It was informal, didn't always involve everyone. Oddly, we had three different pms during this project. We had a hard rule of no email or contact outside of stand-up with them. Two of the three pms were able to 'work' with this setup. The head of airport IT eventually realized after two years of this that we didn't need a PM. We had a rule that if you are doing anything novel in the code base you had to have a conversation about it with at least another dev. We all sat within a few feet of each other in a large private office with a huge whiteboard. We used index cards taped to another dedicated whiteboard for stories and if you couldn't describe the essentials on that, you had to break the story into smaller pieces. We were allowed to build our own machines and use as many monitors as we wanted. Was an Airport billing and tariff system for a large international airport, the accounting department head, director and other users were a couple of office doors away. They rarely missed a stand-up and they had an open policy for any real time questions. Standups were informal discussions, demos, question and answers not typically for status. The status update just took one glance at the cards on the whiteboard. The final system improved revenue by 8% in its first and every following month. The director of Accounting was brought before the Airport Authority Board to explain. Billing disputes and adjustments with airlines went from nine days a month to one. The monthly billing workload went from 18 days to 5. Instead of a senior accountant being the primary user, they were able to offload this to one junior accountant with three years experience. Bug rate was six production bugs, zero incorrect invoices in the first year live. I don't have any data after that. The previous rewrite attempt failed after a three year attempt.
azhenley 3y ago
This bureaucracy grows proportional to company size. It gets much, much worse.
arcbyte 3y ago
Im still unclear how people get bit by this kind of thing. The way ive always operated is that nothing in a Code Review can stop a merge unless its a rule written down before the review or its just a blatantly obvious incorrect miss of requirements or functionality. I love seeing peoples comments about how things can be improved in reviews and learning new approaches or context, but if there's not reference to a written rule I should have known about beforehand anyway, I'm not changing it if I don't agree with it.
- tyleo 3y ago
  I disagree with this. For example, lets say an engineer implements a 1000 line function and another engineer says, 'A similar function already exists. Just add a parameter and change these 2 lines'. I don't think we need to over index on written rules for common sense recommendations like this. I'd be happy to see the first engineer blocking another's PR on the request. In the long run it isn't viable to optimize for shipping, I think a lot of the optimization has to be directed towards maintenance.
  - arcbyte 3y ago
    Common sense is never common. I'm sure you can find something written to block nonsense like that - is it fully unit tested? You do have something written that requires unit tests right? But at the end of the day, if you're on a team with somebody like that then you NEED written rules to deal with that nonsense and the Code Review process is the first step to identify it. The nonsense gets merged the first time, but not before you add a written rule against it going forward. Problem solved.
- ravenstine 3y ago
  > The way ive always operated is that nothing in a Code Review can stop a merge unless its a rule written down before the review or its just a blatantly obvious incorrect miss of requirements or functionality. I've never worked on a team where code reviews worked that way for anyone besides maybe the team lead. Even if nobody uses the GitHub feature to add a blocking review comment, there's always this expectation that nearly every review comment has to be treated as "right" by default, even if it's ultimately based on opinion, and your job as the submitter is to either debate or accept every opinion before merging; this of course means a few changes lines can take days to get merged, unless everyone jumps on a call to have a synchronous code review, which nobody wants to do every single day. The reality is most organizations don't write anything down, and even when they occasionally do, there's a high probability that said documentation is outdated. So while I like the idea of the written rule principle, I don't see it being widely adopted, and there are many more developers who actually believe it's better that every decision just lives in people's heads.
synergy20 3y ago
yet during interviews you're expected to write 50 lines or 100 lines of code with tricky algorithms under pressure in 30-45 minutes from design to implementation to unit tests and corner case considerations, also you must keep talking while coding with your interviewer so you behave like a team player and is collaborating with your future team mate on the fly. all these have nothing to do with real life coding, it's more like a coding monkey stress test to me, you can only perform well by doing daily leetcode for months, it's geared towards young people who has those times to practice. I recall the days I spent two weeks to add 50 lines of code to linux kernel, 95% of the time was to figure out how that kernel subsystem works and where to hook the code, but that matters 0 to nowadays FAANG interview process, the 5% of time for 50 lines code is all that matters.
- crop_rotation 3y ago
  Real world programming and interview programming are like 2 different disciplines at this point.
  - guy98238710 3y ago
    It's so sad and also terrifying when people call this real world programming.
bsuvc 3y ago
It sucks when you say it like the title: "6 days to change 1 line of code", But... The system improved in a few other ways: 1. The setting became configurable via the parameters table instead of being hard coded. 2. Auditing was introduced to track changes to that setting. I'm not trying to defend bureaucracy, because I truly hate that aspect of large organizations, but just point out that additional value was created during those 6 days beyond what was originally set out to be achieved. Incidentally, this is why you have to bake in a set amount of overhead into your estimates, and need to take this red tape into consideration if you do story pointing.
- ebiester 3y ago
  Step 1: determine true priority. How long can this take before it affects jobs? Everyone needs that in mind. If a week won’t affect anyone’s jobs, follow the process or minimally alter it. If people are on furlough because of IT, you have everyone needed in the same room (physical or virtual) until the problem is solved. We don’t have that context here. But if Ed and the entire chain doesn’t have that context, That’s a system failure. The senior would likely just insist on a second ticket to fix it right after if he knew someone’s rent check was on the line. (And if not? That’s also a management issue to solve.)
  - bsuvc 3y ago
    That's a good point. The urgency of this problem was not communicated broadly enough and the impact of the simple fix was not communicated back up the organization so that it could be prioritized correctly. It seems like Ed is just being "helpless" and blaming the organization when maybe he could have raised the issue back up to the president saying "my 1 line of code fix to your problem is being blocked by these people. Can you help?" This was a communication problem as much as anything.
- pif 3y ago
  > It sucks when you say it like the title: "6 days to change 1 line of code" Which just states the fact. > The system improved in a few other ways Which were not required.
karmakaze 3y ago
Very little of this seemed unreasonable, with the exception of: > Pissed off hours spent on Hacker News: 14. I hope it was an exaggeration, but also well within the realm of possibility. This is the only bit that looked like a problem to me: David: It's for Philip. It we don't do this right away, we'll have to have a layoff. Judy: OK, then I'll fill out that section myself and put this on the fast track. 2 days later. If humans understand a change that's in-progress to avoid layoffs, then there's no reason to effect layoffs dogmatically because of a policy which is relying on a laggy process. There's good stuff in here too: David: Forget the queue. Mark it urgent and send it to Ed immediately. 1 hour later. Ed (programmer): On line 1252 of Module ORP572, I changed the hard-coded variable MonthsOfBacklog from "3" to "4"... Shirley (Code Review): It is now against company policy to have any hard-coded variables. You will have to make this a record in the Parameters file. Also, there are 2 old Debug commands, an unassigned variable warning message, and a hard-coded Employee ID that will all have to be fixed before this module can be moved to production. Ed: I don't have access to Marge. Julie: Then contact Joe in IT Security. He'll get you permissions. 2 hours later. Shirley: Your new Parameters record "MonthsOfDemand" needs a better name. The offshore programmers won't understand what this means. Also, it should have an audit trail of changes. And more bad stuff Ed: What policy is that? Shirley: It's not exactly written down anywhere. The offshore team is 3 months late updating the wiki, but I assure you, all new Parameter records must satisfy new naming requirements and keep audit trails. 1 day later: Why would a variable rename take 1 day, perhaps that disgruntled time on HN? This is bad on both the process-side as well as execution. It shouldn't take 2 days to write a test plan as unnecessary as it may seem. Perhaps that's another day spent on HN? Tony (IT Testing): I see 129281 on Marge, but I have no Test Plan. Ed: Just run it the old way and the new way and note the increase in the total on the WorkOrdersHours report. Tony: That's your test plan? No. This affects everything in the factory. I have to have user selected Test Cases, Expected Results, documented Test Runs, and user sign-off. 2 days later: I've read about bad processes and have even worked in worse environments. This story is nowhere near as bad as it gets. At multiple points problems were solved in <= 2 hours by humans, presumably overriding any silly processes.
- add-sub-mul-div 3y ago
  > Why would a variable rename take 1 day, perhaps that disgruntled time on HN? When I work in an environment without bullshit I love my job and get lots done. When I work in an environment like this I hate my job and have no motivation to finish two ticket in the time I could finish one, because it would means twice as much bullshit.
deleted 3y ago
[deleted]
jasmer 3y ago
The people here commenting that this is outrageous have missed the boat a little bit. This 'one line change' is an existential and fundamental effect on the company, it may very well affect operations otherwise. 'One line of code' can easily blow up a system, and the checks are not there to for the 99% of time time we are 'all good' it's for the 1% of the time when there is sketchiness. Someone complaining about 'hard coded variables' is perfectly right to do so. Every single element in this situation is in place for very good reason. Imagine someone wanting to replace the lock in your Airline door, mid flight - oh, we'll just use a 'regular screwdriver' instead of the one designed for the door, who cares!? It's just an airplane! The gripes are misplaced: the solution for this situation probably is to have a war room/crunch room situation to move the issue quickly and correctly through the hurdles so it can be done within the timeframe needed. All of those 'delays in between' were the problem - they did not respect the urgency of the situation.
- deleted 3y ago
  [deleted]
LispSporks22 3y ago
Wow that is Discovery’s engineering teams. The whole mess of it is called “Engineering Excellence”. I hated it.
activitypea 3y ago
This conflates two kinds of slowdowns. Post-implementation stuff like thorough peer reviews and automated testing followed by manual QA is usually a net gain on a long enough timeline. The damning and soul-draining part is pre-implementation -- how many people need to agree that the line of code will be changed and how many people must opine on the change before it can be shot over the Dev | QA wall. The worst place I ever worked at had mandatory "ticket refinement" - a feature or a bug couldn't be worked on until it had been through a "refinement session", in which everyone had to chime in, and the ticket wasn't considered refined until everyone on the team fully agreed with the implementation. Naturally over time, this devolved first into writing pseudo-code in Jira tickets and then into near-production code in a google doc overflowing with comments. Leaving that company I told my manager "this is a team of 4 'senior' engineers, I'd suggest you get one proper senior engineer and 3 typists". Nowadays, you could probably shove minutes from those meetings into copilot and call it a day. Of course, the flipside of this was that comp was about on par with the market, and the perks were top-tier. If you're okay with doing the bare minimum while spending ~20-30 hours a week on total nonsense, it's probably the dream job.
a13o 3y ago
Using your code review process to hold changes ransom until they meet some lofty ever-changing team ideals, is disfunctional. "Upgrade as you go" policies leave long tails of half-completed conversions that make the codebase harder to ramp new devs on. You'll also never finish the conversion because there's no guarantee that product focus will walk every part of the codebase with any regularity. Some product areas get left alone for years and years and years. It's either important enough to cut over to the new policy in one concerted project, or it's not important.
- steveBK123 3y ago
  Yea it’s atrocious as management is basically derelict in planning. They are relying on random unrelated work tasks to trigger these unplanned work time bombs all over the code base . Either some new standard is important and the code should be updated, or not. Just relying on randomness and slowing down urgent work ain’t a plan.
JoeAltmaier 3y ago
Code review starts out with good intentions. But some gatekeeper inevitably sets up shop and starts rejecting everything for trivial reasons. Says they're interested in keeping up 'code quality'. But nothing is worse than keeping buggy code around long after the fix is ready, or delaying features so nobody can try them. I'd recommend a process where comments are allowed but reviewers cannot block commits. Each developer is trusted to be careful, make such changes as are pertinent to the task. Use CI as well and depending on the team it can all go very well.
- black_puppydog 3y ago
  this works well when coverage/test quality is high. which is in itself not something that magically happens if you just let devs move as quickly as whatever manager thinks is necessary right now.
- willseth 3y ago
  Then an engineering leader needs to stop that person. Dysfunction can manifest in many ways, and overzealous reviewing is one of them. Changing the process so people can ignore pathological reviewers is a half measure at best. I’m torn about blocking. I do get the sense that it’s frustrating to get that big red block, so in many cases people ask for changes without blocking - basically a “soft block”. But I’ve dealt with plenty of cases where a PR is so far off the rails (usually a jr. developer), I think it’s appropriate to send a message.
  - Tainnor 3y ago
    For me, it really depends on the other person. There are some devs that I trust to carefully consider my comments (even if they end up disagreeing), so unless something is catastrophically wrong, I will still approve. But there are some other developers who seem content to just routinely (not just sometimes) ignore anything that is not phrased as a command (such as "I think we should test this better"), and I've adopted the habit of not approving such PRs immediately.
- qudat 3y ago
  I abhor “every code change needs a reviewer” rule. They are a massive distraction and don’t necessarily lead to better code.
hmaarrfk 3y ago
This is a story where a one line change of a hardcoded value actually went well. I could image a scenerio where somebody stored the number of months of backlog as a 2 bit value. 0, 1, 2 or 3, you know, to be smart and clever. This may not appear as a problem during testing because it may be hidden many layers down, in some downstream service that is untested. Maybe in some low code automation service.... Changing it to 4 would mean the backlog is 0. Who knows what the consequences might be. Would that service go and cancel all jobs in the production queue? Would it email all customers mentioning their stuff is cancelled? I get that this is a seem gly easy change, but if a change of policy is expressed to the software team as an urgent problem, this seems like the management team needs better planning, and not randomly try to prioritize issues.....
- qxmat 3y ago
  Knight Capital!
  - hmaarrfk 3y ago
    https://www.bugsnag.com/blog/bug-day-460m-loss/ https://www.bugsnag.com/blog/bug-day-460m-loss/ It made me laugh! And cry inside
- twic 3y ago
  There's all sorts of ways it could go wrong. Perhaps the real question is where blame will fall if it does. If the big boss says "I decided to take the risk and push this through, I accept this was a consequence of that", great. If the programmers get beatings, not so great.
  - hmaarrfk 3y ago
    the other thing i notice from the story was that an update on something considered mission Critical was not given an update on within 24 hours. IT should have volunteered the info regarding how far back in the backlog this was classified as soon as that prioritization was made. "Behind 14" and with many people on the testing side occupied is obviously not going to help with "layoff level priority". To me, the classification of "enhancement" just doesn't seem to capture the urgency.
qxmat 3y ago
6 days is pretty reasonable to me because I'm an avid supporter of the release train (miss the release train, wait for the next one). Governance is an important aspect of the software development cycle past the PoC stage. Releases shouldn't lead to an existential crisis.
hermannj314 3y ago
My comment is more a meta commentary on factory workers vs software developers. This company leader is willing to lay off factory worker because of 10% under utilization, he can tweak some variables to increase productivity but ultimately the choice is either full utilization or unemployment. He can do this largely because, I assume, these workers are replaceable and can be rehired during busy season and because the profit generated per employee doesn't allow for any inefficiency. I work as a software developer. Our under utilization has to get well over 90% before anyone is even considering getting rid of us. Many of us are putting in four hour work weeks. No one manages our minutes, our bathroom breaks, etc. We are in a period of major capitalization of software. It won't last forever. Eventually the major infrastructure of the IT world will be built and the industry will shift to a maintenance mode. Most of us won't be needed, we will become replaceable and the profit we generate in maintenance mode will be insignificant to what we see today. Factory workers are usually fired within minutes or hours of detections in low productivity of an individual worker. In our life time, I believe this will start happening with software developers.
- starcraft2wol 3y ago
  > These workers are replaceable and can be rehired during busy season That's exactly the difference. The factory is a system of processes designed to remove decision making and variation from each person. You have to evaluate the degree to which that could be done with your skillset. > in a period of major capitalization of software. It won't last forever. I agree with your basic point-- not every company needs engineers developing novel software all the time. It's more of a boom and a bust creative venture, like producing a movie. If you choose to work in development over IT you should accept that risk. But I don't see why this has time has to be the peak.
gaff33 3y ago
Only 6 days? That's impressive!
habosa 3y ago
Reading this as a problem with code review is just wrong. The problem is that a company let its processes, which are made up internal barriers, get in the way of its principles. Every process needs an escape hatch. Changing something which will prevent layoffs should trigger every escape hatch.
hkon 3y ago
6 days. Hey, that could be considered fast most places!
matt3210 3y ago
The example situation is a bad representation. The process is needed for large changes and the downside is that it happens for single line changes too.
okibry 3y ago
I can relate that.
willsmith72 3y ago
I take issue with increasing a backlog to increase utilisation of resources. When you try to make every part of a process operate at 100% capacity, you risk creating bottlenecks. There's always some variability in how long tasks take. If every part of your system is running at 100% capacity, then any small disruption or variation can cause a backup. That backup then propagates throughout the system, causing delays everywhere.
qsantos 3y ago
Shirley's review was accurate for a non-urgent MR, but was bogging an urgent ticket. Ignoring them would not have increased risk, but the policy is important to manage technical debt. A simple resolution in this case is to move the comments to technical tickets, assign them to Ed for later, and approve the MR.
richk449 3y ago
I’m sympathetic to the pain for the programmer, but I still have so many questions: The months of backlog to build against was hard coded? Changing it requires rebuilding and redeploying the entire software stack that runs the factory? This must be some custom in-house MES? The testing requirements to redeploy that must be extensive right? If that is hard coded, who knows what else is. And if the software doesn’t work right, you end up buying or making the wrong parts?
bluedino 3y ago
A modern version would involve three cloud teams, DevSecInfraOps, and discussions about which resource groups something is in. Also there were 1,280 man-hours of meetings involved here
nojvek 3y ago
The core issue here is reviewer saying “if you change this, you also have to fix other pending issues in codebase”. That should be the point of push back “Great feedback, and appreciate the direction to improve code quality, however the implications of changing Y means we need X, Y, Z approval and it will many days. I am making a tech debt task with what you described and will be done in a follow up PR based on priority and bandwidth. Let’s focus on what it would take to get this surgical PR out.” The biggest lesson I learned is to make focused PRs, and learn to pushback when reviewer asks for a scope creep. Mostly other engineers have been pragmatic. It’s unrelated to number of lines as you can reformat while codebase but not change anything logically, or change some feature flags which could have a huge impact. But ONE FOCUSED CHANGE AT A TIME
- mabbo 3y ago
  An even better solution, imho, is that rules should be automated. New rules are added, and the automation adds rule override comments to every existing violation - which also lets you track them. When you need to rush code out that needs to violate a rule, you add an override comment as well, with your name assigned to fix it. Then over time, you build a culture that wants to fix these rule violations separate from feature development.
- hinkley 3y ago
  Score creepers have no idea the architectural damage they cause. When you get over invested in one block of code, people will try to route around it. Then you get layers of these and pretty soon your code is the moral equivalent of Atlanta, GA, which is notorious for the number of ring roads it has.
- foogazi 3y ago
  > The biggest lesson I learned is to make focused PRs, and learn to pushback when reviewer asks for a scope creep. Whenever that happens just add a TODO ticket. Unblock production and the system is not worse for it
- figassis 3y ago
  Usually I avoid making any improvement that is not directly related to the tasks at hand. Even adding a missing semicolon risks bringing this to the attention of an overzealous reviewer that will then make me follow a rabbit hole of legacy fixes. I’d rather create issues silently so I don’t forget instead of even adding a FIXME or TODO. I think this part of reviews is broken. Resolving tech debt should not be a requirement for completing a task, it should be planned.
ravila4 3y ago
This is a culture issue. If people get paid to look busy, they will find ways to look busy. There's no incentive to close tickets quickly, because it will only result in more work.
Jabbles 3y ago
The president orders a massively impactful change and various processes in the company work to make sure it is safe before launching it. Sounds fine to me. If the president were told "this is risky, we have to verify that nothing will catch fire if we store it for more than 3 months" (or similar) I'm sure they would have agreed to proceed with caution, but still treat the matter as high priority.
- thiht 3y ago
  This is not about safety, this is about ticking boxes. Boxes that make sense, but which are irrelevant compared to the potential impact of the change. For example, replacing the constant with a file parameter is important, but completely out of scope of the PR, and could lead to unexpected impacts. Sounds like there are enough things to QA with this small change that there’s no need to add other sources of potential bugs. The naming of the configuration key is also completely ridiculous and a huge waste of time.
deleted 3y ago
[deleted]
up2isomorphism 3y ago
If this is the case, it is actually better than 90% big companies out there.
some_guy_in_ca 3y ago
Welcome to my world. I deal with this everyday. 25 years in enterprise software for medium to large companies...
btbuildem 3y ago
While Shirley is painted as the main antagonist in this story, I feel like OP made one critical mistake: they cared. When you work in a large organization, any kind of emotional engagement / personal attachment to the work at hand is only going to hurt you.
egberts1 3y ago
Yep. Nine months to change a single buggy line of cache line invalidation in the translation look-aside table handling IMUL opcode in QEMU 4.0. imul eax, [ecx+0x41], 0x10 Entails hundreds of hours of single-stepping through that opcode in Linux kernel using an indirect operand pointing toward its own opcode (self-modifying code). Even the extraordinaire Fabrice Bellard (author of QEMU) admitted that it is broke and did a total rewrite, which fixed tons of other issues. https://github.com/unicorn-engine/unicorn/issues/364 https://github.com/unicorn-engine/unicorn/issues/364

358 points by tempodox 3y ago

shpx 3y ago

Better than waiting a year or more for an open source project to make a release bumping a dependency because no one cares.

tbarbugli 3y ago
I disagree, you can fork and apply whatever change is blocking you to the OSS project.

jacknews 3y ago

Or in other words, artificial/arbitrary management metrics/deadlines force through a possibly dangerous change to factory operations?

yosefk 3y ago
What procedure mitigating the supposed risk of this change was skipped due to management pressure according to TFA? The way I read it, the original change of the constant "3" to "4" was tested, it was observed to have the desired effect, and none of the delays before the coding and the testing by the developer or after it did anything to reduce the risk further.
- jacknews 3y ago
  That would be the (non-existent) tests that should perhaps be run because "This affects everything in the factory." Probably the code still works with this change, but does the factory?
raverbashing 3y ago
Yes Because, in this case, the buck stops with him All deadlines are "artificial" and some have a lot of $$$ attached to them (due to regulatory/market/customer/supplier issues) But I know, a blame culture begets a CYA culture, which begets the opposite of "help me help you" (hence why such a parameter is hardcoded in the code - the issue here is not stopping the ~~orphan~~ developer crushing machine but why does it exist in the first place)
- deleted 3y ago
  [deleted]
- jacknews 3y ago
  A CEO is entirely responsible for the culture of an organization.

crop_rotation 3y ago

That is absolutely true. Most companies have code review process that is totally full of nitpicking and trivial comments. I once suggested replacing that with static analysis tools to remove such comments and make feedback faster in our org, and was told that such code review is necessary for everyone. It helps people get promoted, makes people feel they prevented code issues, keeps the code review metrics looking good for the top level overlords who sit and monitor number of reviewer comments.

javajosh 3y ago
It depends on your team. If you have good juniors, they will relish the feedback. Of course it needs to be substantive. Often junior code will be far more complex and difficult to read than senior code, and code reviews are a chance to get that feedback across. That said, often teams don't give credit for code reviews, or take into account the (sometimes considerable) time required to write them. To some extent, tools are also to blame - I think all teams would do well to pick a system that integrates with the dominant IDE. Doing code reviews in the same space you write code just makes sense; requiring that you use a friggin' webapp is HORRIBLE.
- crop_rotation 3y ago
  The thing is, almost all the trivial comments can be easily automated. That would both give the junior dev quicker feedback before even raising the review, and save time for everyone.
  - javajosh 3y ago
    >Of course it needs to be substantive
  - morelisp 3y ago
    Junior code is not usually more complex because of bad naming conventions and inconsistent styling and related issues that tools can help with, although of course these don't help its readability overall. Rather it usually has control flow that's far too literal a translation of requirements leading to an exponential explosion of paths, insufficient and/or over-eager error handling (often both at the same time), poor representational choices leading to inefficiency or more special cases, and so on. None of these can be effectively automatically detected.

iLoveOncall 3y ago

Not sure I see the point, rules are in place for a reason and it's not like changing 2 lines would have taken 12 days. This seems like it changes something with potentially super-high impact, 6 days from request by the CEO to release to production is a pretty good release time.

crop_rotation 3y ago
It is not a good release time for the described scenario. The company badly needed a change to survive so fixing unrelated issues in the same change doesn't make sense.
- iLoveOncall 3y ago
  I could somewhat agree with that, however it seems like this only added a couple of hours out of the 6 days.
- icegreentea2 3y ago
  The problem (and solution) isn't necessarily to change the process. Assuming good faith and competence when designing the process, all (or similar) of the downsides encountered in the scenario were considered when designing the process, and the cost-benefit came out ahead to add the extra safeguards the layers. As demonstrated by the end of the scenario, the 'solution' was the president to more directly intervene. The failure in this scenario isn't necessarily that the process got in the way, it was that when the President and IT Director kicked off the action, they were not fully aware of the efforts and overrides required to escalate to 'do this now'. A converse failure is that the intent of the President and IT Director was not properly communicated to all stakeholders in the process.
- moralestapia 3y ago
  >The company badly needed a change to survive [...] Lol, nah. This is fiction, no company IRL is going to start laying off people after a few days of noticing a small dip in production. The whole premise of "We own this company but if this code change doesn't go through in X days we will fire people instead" is completely stupid. But it is an entertaining story and it gets the point through that internal bureaucracy gets in the way of many things.

don-code 3y ago

There's another failure mode not described here: "product management doesn't understand the change". I'm in the (perhaps enviable, perhaps unenviable) position of getting a small subset of my marching orders directly from our CTO. Engineering at my company is organized in such a way that, while engineers report to the CTO, product management (part of engineering) does not. When the CTO asks me to priority-override a new feature, or even a small change, I've sometimes gotten into arguments with product management over how this wasn't planned for, and how they need time to adjust schedules and collect requirements before I can begin work. Unfortunately, I can't use the "talk to your boss" excuse, due to that quirk of the reporting structure. One memorable example of this was a feature request which came in hot in mid-June, which I had code complete later that day. We actually shipped it a few days ago. To product management's credit, they _did_ come up with requirements that were not met by my initial change, but I question whether releasing fast and iterating, or releasing correct after four weeks, was better for the customer.

throwaway2990 3y ago
Sounds like a horrible place to work.
icegreentea2 3y ago
Why can't you use the talk to your boss excuse? Does your CTO (and whoever PMs ultimately report to) intend for these types of friction to occur every single time a CTO request appears? Does your CTO notice that these small changes takes extra time to occur? Does your CTO even care?
- jstanley 3y ago
  Because the CTO is not the product manager's boss.
  - icegreentea2 3y ago
    My point is that the product manager also has a boss, and both the CTO and that boss have a shared boss (the CEO). If the CTO and product boss are constantly stepping on each other's toes (or whatever), that's a problem that they either need to solve, just acknowledge as just the cost of doing business. And that ought to be communicated and acknowledged by everyone in the line.

politelemon 3y ago

> Julie: Then contact Joe in IT Security. He'll get you permissions. > 2 hours later. This is completely unrealistic, security teams would never respond so quickly.

seanthemon 3y ago
2 programmer hours, obviously
542458 3y ago
Hah, I have a very different experience where I work. At my org every time I submit a ticket requesting access for somebody to such-and-such a system it typically gets done within a couple minutes regardless of what priority I give. I’ve sometimes wondered if help desk staff snap them up as soon as they come in because they’re fast tickets to close and lets them improve their personal metrics.
switch007 3y ago
Unless it’s them getting a P1 security alert because you ran “npm install”
fargle 3y ago
our security team actually responds even faster. they simply deny all requests automatically, but immediately.
aezart 3y ago
It takes multiple weeks to get someone added to the AD group required for wiki edit permissions.

short_throw 3y ago

I was just waiting to hear that the parameter loading wasn't available in some edge case scenario and code review changes took down the system. But actually this is a story about the process working.

zac23or 3y ago

A few years ago, working on a comment team, I created a function to do an AB test using an AB test team system. Someone else does the implantation, not me. Someone didn't like the function, as they didn't use the company's cache system (the AB team previously explained all the reasons why the cache is not used, etc). I started to participate in several meetings to explain that it does not use the cache system because bla bla bla. A week or two after a few meetings (and a lot of money spent), I got it deployed. It's the same company that created a new meeting to discuss wasted time in all the other meetings.... At the time I thought: This is abnormal, but it is actually an extremely common scenario in many companies.

crop_rotation 3y ago
You might be surprised at how common this is.
jawns 3y ago
> It's the same company that created a new meeting to discuss wasted time in all the other meetings.... I've been in this kind of meeting. It was a 30-minute meeting instituting a rule that meetings should be 25 minutes by default.
- scruple 3y ago
  I sat through that one, too.
duxup 3y ago
I worked for a company with a few thousand employees. Everyone had to spend about 6 hours in meetings about how to have more effective meetings. VPs and managers droned on about how important it was. Rule 1 in the training was to have a detailed agenda. As far as I saw nobody ever followed rule 1 except myself and one other person. I’m glad I work at a small company now.

swader999 3y ago

This is a O(1) operation at this company. Did I pass the interview? I still don't want to work there.

zerr 3y ago

Anecdotal experience: after some years in the team with formal code reviews, moved to the team/company without code reviews. Everyone is free to commit/merge to any branch. When joining the company, I had somewhat mixed feelings about this. Apparently, it felt so refreshing and empowering, I became productive within few days.

jefozabuss 3y ago
sounds like a security and audit nightmare to be honest but I guess it’s an agency or similar with smaller projects
anthomtb 3y ago
A while back, I also worked for a team that did "Catholic Code Reviews" (push and pray). No code review worked very well given the objectives of the team. It was an R&D group who's primary objective was "demo the nifty new thing to the executives". That meant a lot of short notice requests but also a lot of throwaway code. We'd demo the thing, the exec would be like "looks great, no business case" and the repo would never be touched again. Of course, every now and then our thing WOULD get productized and the downstream team would be responsible for turning chicken-scratch code into something production worthy. Those guys hated us with a burning passion.
swader999 3y ago
I've seen this work really well on small teams where there's high degree of trust, ~80% test coverage. It was a no PR process, just merge with master when tests pass, UX(if applicable) demoed successfully to stakeholders and you are satisfied with it. New team members were assigned a mentor that sat beside and frequently paired and reviewed their code for the first 2-3 months. Was a 2.5 year project, went live at 20 months in, was on time and meet budget with more capability than originally scoped. Many days had 2-3 hours of discussion at the whiteboard. It was informal, didn't always involve everyone. Oddly, we had three different pms during this project. We had a hard rule of no email or contact outside of stand-up with them. Two of the three pms were able to 'work' with this setup. The head of airport IT eventually realized after two years of this that we didn't need a PM. We had a rule that if you are doing anything novel in the code base you had to have a conversation about it with at least another dev. We all sat within a few feet of each other in a large private office with a huge whiteboard. We used index cards taped to another dedicated whiteboard for stories and if you couldn't describe the essentials on that, you had to break the story into smaller pieces. We were allowed to build our own machines and use as many monitors as we wanted. Was an Airport billing and tariff system for a large international airport, the accounting department head, director and other users were a couple of office doors away. They rarely missed a stand-up and they had an open policy for any real time questions. Standups were informal discussions, demos, question and answers not typically for status. The status update just took one glance at the cards on the whiteboard. The final system improved revenue by 8% in its first and every following month. The director of Accounting was brought before the Airport Authority Board to explain. Billing disputes and adjustments with airlines went from nine days a month to one. The monthly billing workload went from 18 days to 5. Instead of a senior accountant being the primary user, they were able to offload this to one junior accountant with three years experience. Bug rate was six production bugs, zero incorrect invoices in the first year live. I don't have any data after that. The previous rewrite attempt failed after a three year attempt.

azhenley 3y ago

This bureaucracy grows proportional to company size. It gets much, much worse.

arcbyte 3y ago

Im still unclear how people get bit by this kind of thing. The way ive always operated is that nothing in a Code Review can stop a merge unless its a rule written down before the review or its just a blatantly obvious incorrect miss of requirements or functionality. I love seeing peoples comments about how things can be improved in reviews and learning new approaches or context, but if there's not reference to a written rule I should have known about beforehand anyway, I'm not changing it if I don't agree with it.

tyleo 3y ago
I disagree with this. For example, lets say an engineer implements a 1000 line function and another engineer says, 'A similar function already exists. Just add a parameter and change these 2 lines'. I don't think we need to over index on written rules for common sense recommendations like this. I'd be happy to see the first engineer blocking another's PR on the request. In the long run it isn't viable to optimize for shipping, I think a lot of the optimization has to be directed towards maintenance.
- arcbyte 3y ago
  Common sense is never common. I'm sure you can find something written to block nonsense like that - is it fully unit tested? You do have something written that requires unit tests right? But at the end of the day, if you're on a team with somebody like that then you NEED written rules to deal with that nonsense and the Code Review process is the first step to identify it. The nonsense gets merged the first time, but not before you add a written rule against it going forward. Problem solved.
ravenstine 3y ago
> The way ive always operated is that nothing in a Code Review can stop a merge unless its a rule written down before the review or its just a blatantly obvious incorrect miss of requirements or functionality. I've never worked on a team where code reviews worked that way for anyone besides maybe the team lead. Even if nobody uses the GitHub feature to add a blocking review comment, there's always this expectation that nearly every review comment has to be treated as "right" by default, even if it's ultimately based on opinion, and your job as the submitter is to either debate or accept every opinion before merging; this of course means a few changes lines can take days to get merged, unless everyone jumps on a call to have a synchronous code review, which nobody wants to do every single day. The reality is most organizations don't write anything down, and even when they occasionally do, there's a high probability that said documentation is outdated. So while I like the idea of the written rule principle, I don't see it being widely adopted, and there are many more developers who actually believe it's better that every decision just lives in people's heads.

synergy20 3y ago

yet during interviews you're expected to write 50 lines or 100 lines of code with tricky algorithms under pressure in 30-45 minutes from design to implementation to unit tests and corner case considerations, also you must keep talking while coding with your interviewer so you behave like a team player and is collaborating with your future team mate on the fly. all these have nothing to do with real life coding, it's more like a coding monkey stress test to me, you can only perform well by doing daily leetcode for months, it's geared towards young people who has those times to practice. I recall the days I spent two weeks to add 50 lines of code to linux kernel, 95% of the time was to figure out how that kernel subsystem works and where to hook the code, but that matters 0 to nowadays FAANG interview process, the 5% of time for 50 lines code is all that matters.

crop_rotation 3y ago
Real world programming and interview programming are like 2 different disciplines at this point.
- guy98238710 3y ago
  It's so sad and also terrifying when people call this real world programming.

bsuvc 3y ago

It sucks when you say it like the title: "6 days to change 1 line of code", But... The system improved in a few other ways: 1. The setting became configurable via the parameters table instead of being hard coded. 2. Auditing was introduced to track changes to that setting. I'm not trying to defend bureaucracy, because I truly hate that aspect of large organizations, but just point out that additional value was created during those 6 days beyond what was originally set out to be achieved. Incidentally, this is why you have to bake in a set amount of overhead into your estimates, and need to take this red tape into consideration if you do story pointing.

ebiester 3y ago
Step 1: determine true priority. How long can this take before it affects jobs? Everyone needs that in mind. If a week won’t affect anyone’s jobs, follow the process or minimally alter it. If people are on furlough because of IT, you have everyone needed in the same room (physical or virtual) until the problem is solved. We don’t have that context here. But if Ed and the entire chain doesn’t have that context, That’s a system failure. The senior would likely just insist on a second ticket to fix it right after if he knew someone’s rent check was on the line. (And if not? That’s also a management issue to solve.)
- bsuvc 3y ago
  That's a good point. The urgency of this problem was not communicated broadly enough and the impact of the simple fix was not communicated back up the organization so that it could be prioritized correctly. It seems like Ed is just being "helpless" and blaming the organization when maybe he could have raised the issue back up to the president saying "my 1 line of code fix to your problem is being blocked by these people. Can you help?" This was a communication problem as much as anything.
pif 3y ago
> It sucks when you say it like the title: "6 days to change 1 line of code" Which just states the fact. > The system improved in a few other ways Which were not required.

karmakaze 3y ago

Very little of this seemed unreasonable, with the exception of: > Pissed off hours spent on Hacker News: 14. I hope it was an exaggeration, but also well within the realm of possibility. This is the only bit that looked like a problem to me: David: It's for Philip. It we don't do this right away, we'll have to have a layoff. Judy: OK, then I'll fill out that section myself and put this on the fast track. 2 days later. If humans understand a change that's in-progress to avoid layoffs, then there's no reason to effect layoffs dogmatically because of a policy which is relying on a laggy process. There's good stuff in here too: David: Forget the queue. Mark it urgent and send it to Ed immediately. 1 hour later. Ed (programmer): On line 1252 of Module ORP572, I changed the hard-coded variable MonthsOfBacklog from "3" to "4"... Shirley (Code Review): It is now against company policy to have any hard-coded variables. You will have to make this a record in the Parameters file. Also, there are 2 old Debug commands, an unassigned variable warning message, and a hard-coded Employee ID that will all have to be fixed before this module can be moved to production. Ed: I don't have access to Marge. Julie: Then contact Joe in IT Security. He'll get you permissions. 2 hours later. Shirley: Your new Parameters record "MonthsOfDemand" needs a better name. The offshore programmers won't understand what this means. Also, it should have an audit trail of changes. And more bad stuff Ed: What policy is that? Shirley: It's not exactly written down anywhere. The offshore team is 3 months late updating the wiki, but I assure you, all new Parameter records must satisfy new naming requirements and keep audit trails. 1 day later: Why would a variable rename take 1 day, perhaps that disgruntled time on HN? This is bad on both the process-side as well as execution. It shouldn't take 2 days to write a test plan as unnecessary as it may seem. Perhaps that's another day spent on HN? Tony (IT Testing): I see 129281 on Marge, but I have no Test Plan. Ed: Just run it the old way and the new way and note the increase in the total on the WorkOrdersHours report. Tony: That's your test plan? No. This affects everything in the factory. I have to have user selected Test Cases, Expected Results, documented Test Runs, and user sign-off. 2 days later: I've read about bad processes and have even worked in worse environments. This story is nowhere near as bad as it gets. At multiple points problems were solved in <= 2 hours by humans, presumably overriding any silly processes.

add-sub-mul-div 3y ago
> Why would a variable rename take 1 day, perhaps that disgruntled time on HN? When I work in an environment without bullshit I love my job and get lots done. When I work in an environment like this I hate my job and have no motivation to finish two ticket in the time I could finish one, because it would means twice as much bullshit.

deleted 3y ago

[deleted]

jasmer 3y ago

The people here commenting that this is outrageous have missed the boat a little bit. This 'one line change' is an existential and fundamental effect on the company, it may very well affect operations otherwise. 'One line of code' can easily blow up a system, and the checks are not there to for the 99% of time time we are 'all good' it's for the 1% of the time when there is sketchiness. Someone complaining about 'hard coded variables' is perfectly right to do so. Every single element in this situation is in place for very good reason. Imagine someone wanting to replace the lock in your Airline door, mid flight - oh, we'll just use a 'regular screwdriver' instead of the one designed for the door, who cares!? It's just an airplane! The gripes are misplaced: the solution for this situation probably is to have a war room/crunch room situation to move the issue quickly and correctly through the hurdles so it can be done within the timeframe needed. All of those 'delays in between' were the problem - they did not respect the urgency of the situation.

deleted 3y ago
[deleted]

LispSporks22 3y ago

Wow that is Discovery’s engineering teams. The whole mess of it is called “Engineering Excellence”. I hated it.

activitypea 3y ago

This conflates two kinds of slowdowns. Post-implementation stuff like thorough peer reviews and automated testing followed by manual QA is usually a net gain on a long enough timeline. The damning and soul-draining part is pre-implementation -- how many people need to agree that the line of code will be changed and how many people must opine on the change before it can be shot over the Dev | QA wall. The worst place I ever worked at had mandatory "ticket refinement" - a feature or a bug couldn't be worked on until it had been through a "refinement session", in which everyone had to chime in, and the ticket wasn't considered refined until everyone on the team fully agreed with the implementation. Naturally over time, this devolved first into writing pseudo-code in Jira tickets and then into near-production code in a google doc overflowing with comments. Leaving that company I told my manager "this is a team of 4 'senior' engineers, I'd suggest you get one proper senior engineer and 3 typists". Nowadays, you could probably shove minutes from those meetings into copilot and call it a day. Of course, the flipside of this was that comp was about on par with the market, and the perks were top-tier. If you're okay with doing the bare minimum while spending ~20-30 hours a week on total nonsense, it's probably the dream job.

a13o 3y ago

Using your code review process to hold changes ransom until they meet some lofty ever-changing team ideals, is disfunctional. "Upgrade as you go" policies leave long tails of half-completed conversions that make the codebase harder to ramp new devs on. You'll also never finish the conversion because there's no guarantee that product focus will walk every part of the codebase with any regularity. Some product areas get left alone for years and years and years. It's either important enough to cut over to the new policy in one concerted project, or it's not important.

steveBK123 3y ago
Yea it’s atrocious as management is basically derelict in planning. They are relying on random unrelated work tasks to trigger these unplanned work time bombs all over the code base . Either some new standard is important and the code should be updated, or not. Just relying on randomness and slowing down urgent work ain’t a plan.

JoeAltmaier 3y ago

Code review starts out with good intentions. But some gatekeeper inevitably sets up shop and starts rejecting everything for trivial reasons. Says they're interested in keeping up 'code quality'. But nothing is worse than keeping buggy code around long after the fix is ready, or delaying features so nobody can try them. I'd recommend a process where comments are allowed but reviewers cannot block commits. Each developer is trusted to be careful, make such changes as are pertinent to the task. Use CI as well and depending on the team it can all go very well.

black_puppydog 3y ago
this works well when coverage/test quality is high. which is in itself not something that magically happens if you just let devs move as quickly as whatever manager thinks is necessary right now.
willseth 3y ago
Then an engineering leader needs to stop that person. Dysfunction can manifest in many ways, and overzealous reviewing is one of them. Changing the process so people can ignore pathological reviewers is a half measure at best. I’m torn about blocking. I do get the sense that it’s frustrating to get that big red block, so in many cases people ask for changes without blocking - basically a “soft block”. But I’ve dealt with plenty of cases where a PR is so far off the rails (usually a jr. developer), I think it’s appropriate to send a message.
- Tainnor 3y ago
  For me, it really depends on the other person. There are some devs that I trust to carefully consider my comments (even if they end up disagreeing), so unless something is catastrophically wrong, I will still approve. But there are some other developers who seem content to just routinely (not just sometimes) ignore anything that is not phrased as a command (such as "I think we should test this better"), and I've adopted the habit of not approving such PRs immediately.
qudat 3y ago
I abhor “every code change needs a reviewer” rule. They are a massive distraction and don’t necessarily lead to better code.

hmaarrfk 3y ago

This is a story where a one line change of a hardcoded value actually went well. I could image a scenerio where somebody stored the number of months of backlog as a 2 bit value. 0, 1, 2 or 3, you know, to be smart and clever. This may not appear as a problem during testing because it may be hidden many layers down, in some downstream service that is untested. Maybe in some low code automation service.... Changing it to 4 would mean the backlog is 0. Who knows what the consequences might be. Would that service go and cancel all jobs in the production queue? Would it email all customers mentioning their stuff is cancelled? I get that this is a seem gly easy change, but if a change of policy is expressed to the software team as an urgent problem, this seems like the management team needs better planning, and not randomly try to prioritize issues.....

qxmat 3y ago
Knight Capital!
- hmaarrfk 3y ago
  https://www.bugsnag.com/blog/bug-day-460m-loss/ https://www.bugsnag.com/blog/bug-day-460m-loss/ It made me laugh! And cry inside
twic 3y ago
There's all sorts of ways it could go wrong. Perhaps the real question is where blame will fall if it does. If the big boss says "I decided to take the risk and push this through, I accept this was a consequence of that", great. If the programmers get beatings, not so great.
- hmaarrfk 3y ago
  the other thing i notice from the story was that an update on something considered mission Critical was not given an update on within 24 hours. IT should have volunteered the info regarding how far back in the backlog this was classified as soon as that prioritization was made. "Behind 14" and with many people on the testing side occupied is obviously not going to help with "layoff level priority". To me, the classification of "enhancement" just doesn't seem to capture the urgency.

qxmat 3y ago

6 days is pretty reasonable to me because I'm an avid supporter of the release train (miss the release train, wait for the next one). Governance is an important aspect of the software development cycle past the PoC stage. Releases shouldn't lead to an existential crisis.

hermannj314 3y ago

My comment is more a meta commentary on factory workers vs software developers. This company leader is willing to lay off factory worker because of 10% under utilization, he can tweak some variables to increase productivity but ultimately the choice is either full utilization or unemployment. He can do this largely because, I assume, these workers are replaceable and can be rehired during busy season and because the profit generated per employee doesn't allow for any inefficiency. I work as a software developer. Our under utilization has to get well over 90% before anyone is even considering getting rid of us. Many of us are putting in four hour work weeks. No one manages our minutes, our bathroom breaks, etc. We are in a period of major capitalization of software. It won't last forever. Eventually the major infrastructure of the IT world will be built and the industry will shift to a maintenance mode. Most of us won't be needed, we will become replaceable and the profit we generate in maintenance mode will be insignificant to what we see today. Factory workers are usually fired within minutes or hours of detections in low productivity of an individual worker. In our life time, I believe this will start happening with software developers.

starcraft2wol 3y ago
> These workers are replaceable and can be rehired during busy season That's exactly the difference. The factory is a system of processes designed to remove decision making and variation from each person. You have to evaluate the degree to which that could be done with your skillset. > in a period of major capitalization of software. It won't last forever. I agree with your basic point-- not every company needs engineers developing novel software all the time. It's more of a boom and a bust creative venture, like producing a movie. If you choose to work in development over IT you should accept that risk. But I don't see why this has time has to be the peak.

gaff33 3y ago

Only 6 days? That's impressive!

habosa 3y ago

Reading this as a problem with code review is just wrong. The problem is that a company let its processes, which are made up internal barriers, get in the way of its principles. Every process needs an escape hatch. Changing something which will prevent layoffs should trigger every escape hatch.

hkon 3y ago

6 days. Hey, that could be considered fast most places!

matt3210 3y ago

The example situation is a bad representation. The process is needed for large changes and the downside is that it happens for single line changes too.

okibry 3y ago

I can relate that.

willsmith72 3y ago

I take issue with increasing a backlog to increase utilisation of resources. When you try to make every part of a process operate at 100% capacity, you risk creating bottlenecks. There's always some variability in how long tasks take. If every part of your system is running at 100% capacity, then any small disruption or variation can cause a backup. That backup then propagates throughout the system, causing delays everywhere.

qsantos 3y ago

Shirley's review was accurate for a non-urgent MR, but was bogging an urgent ticket. Ignoring them would not have increased risk, but the policy is important to manage technical debt. A simple resolution in this case is to move the comments to technical tickets, assign them to Ed for later, and approve the MR.

richk449 3y ago

I’m sympathetic to the pain for the programmer, but I still have so many questions: The months of backlog to build against was hard coded? Changing it requires rebuilding and redeploying the entire software stack that runs the factory? This must be some custom in-house MES? The testing requirements to redeploy that must be extensive right? If that is hard coded, who knows what else is. And if the software doesn’t work right, you end up buying or making the wrong parts?

bluedino 3y ago

A modern version would involve three cloud teams, DevSecInfraOps, and discussions about which resource groups something is in. Also there were 1,280 man-hours of meetings involved here

nojvek 3y ago

The core issue here is reviewer saying “if you change this, you also have to fix other pending issues in codebase”. That should be the point of push back “Great feedback, and appreciate the direction to improve code quality, however the implications of changing Y means we need X, Y, Z approval and it will many days. I am making a tech debt task with what you described and will be done in a follow up PR based on priority and bandwidth. Let’s focus on what it would take to get this surgical PR out.” The biggest lesson I learned is to make focused PRs, and learn to pushback when reviewer asks for a scope creep. Mostly other engineers have been pragmatic. It’s unrelated to number of lines as you can reformat while codebase but not change anything logically, or change some feature flags which could have a huge impact. But ONE FOCUSED CHANGE AT A TIME

mabbo 3y ago
An even better solution, imho, is that rules should be automated. New rules are added, and the automation adds rule override comments to every existing violation - which also lets you track them. When you need to rush code out that needs to violate a rule, you add an override comment as well, with your name assigned to fix it. Then over time, you build a culture that wants to fix these rule violations separate from feature development.
hinkley 3y ago
Score creepers have no idea the architectural damage they cause. When you get over invested in one block of code, people will try to route around it. Then you get layers of these and pretty soon your code is the moral equivalent of Atlanta, GA, which is notorious for the number of ring roads it has.
foogazi 3y ago
> The biggest lesson I learned is to make focused PRs, and learn to pushback when reviewer asks for a scope creep. Whenever that happens just add a TODO ticket. Unblock production and the system is not worse for it
figassis 3y ago
Usually I avoid making any improvement that is not directly related to the tasks at hand. Even adding a missing semicolon risks bringing this to the attention of an overzealous reviewer that will then make me follow a rabbit hole of legacy fixes. I’d rather create issues silently so I don’t forget instead of even adding a FIXME or TODO. I think this part of reviews is broken. Resolving tech debt should not be a requirement for completing a task, it should be planned.

ravila4 3y ago

This is a culture issue. If people get paid to look busy, they will find ways to look busy. There's no incentive to close tickets quickly, because it will only result in more work.

Jabbles 3y ago

The president orders a massively impactful change and various processes in the company work to make sure it is safe before launching it. Sounds fine to me. If the president were told "this is risky, we have to verify that nothing will catch fire if we store it for more than 3 months" (or similar) I'm sure they would have agreed to proceed with caution, but still treat the matter as high priority.

thiht 3y ago
This is not about safety, this is about ticking boxes. Boxes that make sense, but which are irrelevant compared to the potential impact of the change. For example, replacing the constant with a file parameter is important, but completely out of scope of the PR, and could lead to unexpected impacts. Sounds like there are enough things to QA with this small change that there’s no need to add other sources of potential bugs. The naming of the configuration key is also completely ridiculous and a huge waste of time.

deleted 3y ago

[deleted]

up2isomorphism 3y ago

If this is the case, it is actually better than 90% big companies out there.

some_guy_in_ca 3y ago

Welcome to my world. I deal with this everyday. 25 years in enterprise software for medium to large companies...

btbuildem 3y ago

While Shirley is painted as the main antagonist in this story, I feel like OP made one critical mistake: they cared. When you work in a large organization, any kind of emotional engagement / personal attachment to the work at hand is only going to hurt you.

egberts1 3y ago

Yep. Nine months to change a single buggy line of cache line invalidation in the translation look-aside table handling IMUL opcode in QEMU 4.0. imul eax, [ecx+0x41], 0x10 Entails hundreds of hours of single-stepping through that opcode in Linux kernel using an indirect operand pointing toward its own opcode (self-modifying code). Even the extraordinaire Fabrice Bellard (author of QEMU) admitted that it is broke and did a total rewrite, which fixed tons of other issues. https://github.com/unicorn-engine/unicorn/issues/364 https://github.com/unicorn-engine/unicorn/issues/364