8 ms·
> A healthy engineering team is constantly building trust. Rewriting your teammate’s code without a discussion is a huge blow to your ability to effectively co
by temac 7y ago
> A healthy engineering team is constantly building trust. Rewriting your teammate’s code without a discussion is a huge blow to your ability to effectively collaborate on a codebase together.
I'm against the idea that people should be attached to "their" code (that is: the code they wrote). Now I also understand that humans that humans, but the priority should be to make them evolve toward more detachment from their work and acceptance that what they design "can" (actually will) be imperfect, rather than avoiding upsetting them even when it would not be justified to be upset. Plus one essential purpose of source control is to be able to make changes, and revert them if they were "wrong"; or for any other purpose. Maybe propose a patch instead of committing directly, but that's really a cultural matter about how projects are organized.
I don't want to ask of permission for an improvement. If there is a need to have formal authorization of maintainers responsible for parts of the code, they setup just that. Otherwise, I'm certainly going to improve "your" code, in some rare cases without even telling you (that's not a goal in itself to do it behind your back, obviously, and I also value collaboration -- but that should not be a problem). The question that remains is: is it really an improvement. If you are not sure, then maybe don't commit. If you are, do it (following your local rules), and if it ends up being a mistake, yes, it will be reverted, and so what?
You should not take issue of your work being reverted (for good reasons), like other people should not take issue of "their" code being modified. Better ask for forgiveness than permission.
> For example, we later needed many special cases and behaviors for different handles on different shapes. My abstraction would have to become several times more convoluted to afford that, whereas with the original “messy” version such changes stayed easy as cake.
Then re-expand the code later.
- lonesword 7y ago> I'm against the idea that people should be attached to "their" code (that is: the code they wrote). Can people _reaaally_ detach themselves from the code they write? After all, programming is a somewhat creative profession. I can't imagine painters and writers detaching themselves from their work. I wonder how doctors and surgeons cope with this. Their work directly affects human lives and I can imagine a mistake weighing down heavily on them. However, I'd like to imagine that my doctor doesn't "detach" himself/herself from their work. > I don't want to ask of permission for an improvement If I check in working code that passed a code review, and someone else just decides to overwrite it with their version of aesthetically pleasing code (i.e not a bugfix) without so much as shooting an email in my direction, I would be outright offended. Any changes other than a critical bugfix/performance improvement can wait until you get a chance to speak with the original dev - this is common courtesy, not bureaucratic red tape.
- kortilla 7y ago> I would be outright offended. Get over it. It is not your code (assuming you are working for a company). If some other developer wants to waste time making changes that don’t do anything, that’s between them and their manager. Would you be “outright offended” if your code got changed after you left?
- danabramov 7y agoI think there’s a balance. There’s a difference between gradually improving something, or improving it at some point later — and literally rewriting 100% of code someone has just landed over the night.
- slavik81 7y agoI agree with you. To me, the action felt wrong when he described it. I wasn't sure why at first, but I think perhaps because the work was not in service to any actual task. If he was in charge of writing the next piece of functionality for that code, it would be poor teamwork to completely refactor it without discussing the reasons for the design, but at least it would have been within his area of responsibility. This is also the reason why I like having code review. The review would have been a good place to raise his concerns. They could have had their eventual conversion earlier, before feelings were hurt or wasted efforts were made. It would even help in the opposite case, where the author's code was legitimately poor and the reviewer had good suggestions. It's not a panacea, but I find code review helpful. In my experience, review was the time when a lot of knowledge-sharing occurred. Some might feel that only senior developers should review, but as a junior I learned a lot about why the author made the choices they did. I learned how people expected the code to change, about language features or pitfalls, etc., and it helped me grow as a developer.
- cellularmitosis 7y agoThis actually happened to me, (and his rewrite didn’t even work!). Having witnessed this first-hand, I can say that the impact of the code change was absolutely dwarfed by the lost trust. Unsurprisingly, I would come to find out that this engineer had what I would call the opposite of soft skills, and the notion of a “this irks me so I rewrote your code” has become a giant red flag for me.
- kortilla 7y agoDid you lose trust because the code broke (a legit reason to lose trust) or because your code was changed without kissing your ring (a problem with you being too attached to your code emotionally).
- rubatuga 7y agoI mean, it's nice to talk about ideals, but it's also highly unrealistic that people are able to separate the code they write and what is says about them. Especially if you are part of a newly formed team, you must make sure that there is trust, respect, and clear communication, because this is the stage where most misconceptions occur. Upon building a successful foundation, you can test new team dynamics such as changes in communication that tradeoff that initial safety for more efficiency.
- yxhuvud 7y agoYou want trust, but you also want code review. The example lacked both.
- the_unproven 7y agoI also think it's fine to change the code someone wrote. Just because someone wrote it, doesn't mean it's the right way to do it. I often find myself rewriting the code, it's the natural process of code evolution. It just feels that it should be more readable, efficient etc. Although, if the change is essential or it requires more pair of eyes, I'll just make a PR(MR) and let the people review it.
- GordonS 7y agoI think a better process is to discuss with the other dev, talking through how you think it could be improved. They (or sometimes you!) can learn something that will lead to improved in the future. It's a bit like that analogy about giving a man a fish vs a fishing rod.
- mannykannot 7y agoThere is a delicate balance: a sense of ownership can encourage pride and responsibility, but it can also impede cooperation. > Better ask for forgiveness than permission. Not in this case; you are likely to piss people off, especially if there is a consequence you have overlooked.
- skybrian 7y agoIn an environment where all code must be reviewed, when you make a change like this you send it to the original owner for review. Or at least someone on the same team, if they're not available. "Don't be too attached to your code" means trying to review improvements to the code you just wrote objectively. Can you accept changes with good grace? But you can't get to the point where you have a smooth-working team if you have philosophical differences about what improvement looks like! If one person's improvement is another person's regression then those difference need to be worked out and some kind of synthesis agreed to. This can take a lot of time, but it's necessary work to get to that ideal. On the other hand, a lot of work gets done by teams that are not smooth-running teams that are in philosophical agreement. It may even be a good thing to have a diversity of opinion and avoid group-think? You have to be able to accept some messiness though.
- tcbawo 7y agoThe quality of a team is the quality of the communication. If the architecture and design isn't well communicated, you get people going off in the wrong direction, or polluting a clean design. Also, people are most attached to their code based on their time investment. If you trash someone's code/changes, you trash their time, which could have been more efficiently decided with a conversation ahead of time. If you find yourself surprised by design changes, find design flaws in a code review, or have different definitions of "improvement", the communication in your company may be lacking.
- jt2190 7y ago> You should not take issue of your work being reverted (for good reasons), like other people should not take issue of "their" code being modified. Better ask for forgiveness than permission. Changing one developers _working code_ after they’ve invested a significant amount of time into without discussing it with the team first it is to basically heap on a number of unwritten requirements and also decide that the schedule allows for addressing them. If removing repetitive code is a requirement then the team needs to be informed that code will be reviewed for repetition, and introduced to various techniques for spotting and removing repetitive code. The schedule also needs to be adjusted to allow for this additional work. And if the rewritten code is so superior than perhaps there needs to be a discussion about whether there should even be a team, or whether the other people should be let go and all work assigned to the developer who can do the work “properly” the first time. After all, what’s the point in having one developer’s work constantly rewritten by another?
- habitue 7y ago> Changing one developers _working code_ after they’ve invested a significant amount of time How much time they invested is irrelevant. There are lots of times where someone is wrestling with something for so long they just want to get it done and don't want to look at it anymore. Many times it's trivial for someone fresh to tidy it up > If removing repetitive code is a requirement then the team needs to be informed that code will be reviewed for repetition This focus on "requirements" is only really trotted out when someone doesn't like someone changing their code and is looking for a defense. In reality, coding is super subjective, there are dozens of aesthetic judgement calls everyone has to make. Saying something "wasn't a requirement" basically means "fuck off I don't like you touching my code". There's no way to codify concrete requirements for handling every possible way code can be improved. If devs are throwing requirements at each other, that's a culture problem, not a spec problem
- roenxi 7y ago> How much time they invested is irrelevant. That is a central point to gp's argument though. The amount of time does matter, because it affects the schedule and hence triggers management decisions. > ... aesthetic judgement calls everyone has to make. Saying something "wasn't a requirement" basically means "fuck off I don't like you touching my code". If you believe that then you really shouldn't be tidying up other people's code. Why are you making people unhappy over something that you think is an aesthetic judgement call? Let them be happy.
- Jasper_ 7y agoIf someone spends a week or two writing a patch and you come in and rewrite it in an evening, that, in and of itself, is telling me something: You think your teammate is a worse coder than you, given you were able to solve it with "cleaner" code. You assumed that your solution was better, without talking to the person who authored it to see if they did things that way for a reason. This could have been solved with a "why did you do things this way", and maybe you would have learned a bit about the thought process behind it, or maybe you would have gotten "yeah this could probably be better, go ahead and clean it up". In a lot of cases, I definitely get the latter, but I always ask first, because if they had a reason to do things a certain way, they probably don't want someone stomping on a feature they're actively working on.
- jrochkind1 7y ago> If someone spends a week or two writing a patch and you come in and rewrite it in an evening, that, in and of itself, is telling me... Or, what it might be telling you is the second person wouldn't have been able to clean it up in an evening without the first person having already spent a week or two on it. The second person was building on the first. Just because it was (hypothetically) less lines of code doesn't mean it could have been written without starting with the more lines of code and refactoring; it often takes more time/steps to get to fewer lines of code. But don't get me wrong, I'm in favor of colleagues discussing code, not just changing each other's code without discussing it.
- sinsterizme 7y agoThis blows my mind. If I had to discuss with my teammates every time I changed their code I would be way less productive. Everyone’s a little bit watched to the code they write, but iterative improvements and bumping the quality of the codebase is way more valuable than risking a fragile ego. It’s the teams code, we all benefit from improving it.
- jrochkind1 7y agoYou can certainly go too far in either direction (talking or not talking), but to the extent talking matters when it does, it's not about "ego", it's about 1) "What did they know that I don't" and 2) Building shared understanding of the context and decisions and architecture. As the OP says "A healthy engineering team is constantly building trust." Sometimes you already have enough trust and shared understanding to not need a discussion, other times a discussion is good.
- cle 7y agoThe point isn't to stroke your teammates' egos. It's to find out why the code is written that way, and to build consensus around a different way of writing it. Consensus building is a great way to improve the cohesiveness of the team. It lets teammates operate with a set up implicit, shared assumptions, which means they can spend more time on business problems and less time on these kinds of discussions. Generally, lone wolves worsen the team dynamics, because they reduce team cohesion.
- blazespin 7y agoRewriting code is fine, but you really better know what you are doing. Doing it without first talking to the engineer is insanely risky.
- slavik81 7y ago> I'm against the idea that people should be attached to "their" code I agree with this, but it goes both ways. The codebase belongs to the team, not any individual developer. Making changes for the sole purpose of making the team's code correspond to your personal preferences is the opposite of professional detachment.
- vgrafe 7y agoThis is about ownership. I agree there's no need to be personally attached to your own code. I have no problem with someone taking ownership of my work (understand refactoring/abstracting the crap out of it for the sake of Engineering). But the one day that person leaves the team, you have no choice but deal with that person's mental model, as nice or convoluted it was while refactoring. I've had this experience in every team I've worked with...
- latchkey 7y agoOf course, we all have had that experience! So you have a choice... realize it is going to happen and learn to deal with it in a productive manner, or just continue to get upset about it. I know which choice I'm working towards...
- vgrafe 7y agoAre you suggesting that a dev acting like this without consulting the team should be considered normal/expected? Not sure how to address a move which is unproductive in its nature "in a productive manner" unless being proactive and preventing those rewrites to happen, unless needed and discussed with the team. But in my personal experience the team's decisions were always ignored by the coder in question. edit: I should remind for context, I have no problem with the rewrite, but I have one with the team being handed over-abstracted code; I found that this happens mostly with more junior profiles, the same that don't stay around for long.
- latchkey 7y agoSince this is a fairly common dev experience (rewriting code), that the team meet together to set a policy as a group. Similar to code format standards, why not talk about code maintenance standards?
- vgrafe 7y agoI get you now, and this is definitely an approach I agree with. I guess I was ranting about my previous experiences... since the last departure I encourage the rest of the team to plan and discuss such changes.
- vez- 7y agoI disagree that people should NOT be attached to their code. Having a sense of ownership for what you write can lead to higher quality systems where people are willing to stand up and fight for what they believe is higher quality. BUT this importantly depends on the ability to compromise, admit being wrong, and change based on new information from the people who have a sense of ownership over their parts of the codebase. Like you said.
- untoreh 7y agotragedy of the commons for code :)
- kortilla 7y agoPeople that get really attached to their code tend to be the people not smart enough for you to want them fighting for their opinions. Good engineers promote good ideas and general approaches to code base structure. They don’t give a shit if people go in and make changes as long as they don’t compromise the whole architecture.
- wutbrodo 7y ago> the priority should be to make them evolve toward more detachment from their work and acceptance that what they design "can" (actually will) be imperfect In an ideal world, sure, but I've worked with enough people varying from professional-but-proud[1] to emotional-maturity-of-a-child to understand that this _shouldn't_ be prioritized, on grounds of feasibility. You may as well say "we don't need reasonable workdays, we need to teach employees to jettison their biological need for sleep". I don't think it's _impossible_ to get people to be fully detached from their code, but I think that it's only possible in fairly narrow situations (perhaps in a small, tight-knit, talented team culture). Trying to get it to work in a general context is usually much more effort (and lower chance of success) than meeting it halfway with tweaks to communication style and process. [1] To be fair, I'm quite sure I've slipped into this mode on occasion myself
- avl999 7y ago> I don't want to ask of permission for an improvement. If there is a need to have formal authorization of maintainers responsible for parts of the code, they setup just that. Otherwise, I'm certainly going to improve "your" code, in some rare cases without even telling you (that's not a goal in itself to do it behind your back, obviously, and I also value collaboration -- but that should not be a problem). The question that remains is: is it really an improvement. If you are not sure, then maybe don't commit. If you are, do it (following your local rules), and if it ends up being a mistake, yes, it will be reverted, and so what? The issue remains that "improvements" and "good code" are too a large extent subjective. And the issue with doing a major refactor of code someone else wrote just days before on your own without consulting the original author or the team late at night as is described in the article (and doing it repeatedly) is showing poor social skills at best and being actively dangerous at worst. Here are some reasons that you should consult the team and or the original author before doing a large scale refactor of recent code: * Start off with the principle of charity and assume that the person who wrote that code and the people who reviewed it aren't complete idiots and you aren't some kind of savant who is able to see something they didn't while working on it for a much longer period of time. * Since that person has been working on it for days they might have a perfectly valid reason for writing the code the way they did. One example is that they are working with the stakeholder and this is the first of a series of changes to deliver specific functionality and the code is the way it is because it facilitates those upcoming changes that might only be days away. * If you have a habit of doing this and your changes getting reverted "ask forgiveness than permission" as you said, the team will begin to lose trust in you and it is bad for team morale. Consulting people before you make a huge change to their recent code makes them feel part of the process as opposed to something that is being done around them with them as spectators. On any team where that happens with any degree of frequency is not going to be a happy team in the long run. And managers will prioritize a happy team with high morale as opposed to a "rockstar refactorer" on the team who is impacting morale regardless of whether their refactorings are error free or not. * It is fundamentally against the spirit of collaboration. When someone releases some new code there is a degree of inherent collaboration in there, it might involve multiple developers, stakeholders, implied functionality that was not part of the original story and at the very least the code reviewer. You unilaterally making a change without even taking 10 mins of your time to discuss it with them is impolite at best and wasting company resources at worst if your "improvement" ends up being reverted and all that work and time could have been saved had you bothered to have a 10 min convo before starting.
- watwut 7y agoHonestly, no attachment to my code demotivates me. It makes me less responsible for outcome and helpless. It makes it much harder to prove I know what I am doing. Since you modify things however you please despite me not agreeing, I am not the one at fault for maintenance issues. It also means that keeping conventions requires constant negotiation and frankly, if I would enjoy people trying to dominate me or me dominate them I would become manager. And yes, it should not be about that, but it is about that with a lot of guys.
- shrimpx 7y ago> I don't want to ask of permission for an improvement. If the team works in branches and uses PRs this is a non-issue, because stakeholders will have a chance to react. It would be an issue if I went to work one morning to find a system I wrote rewritten without my having a chance to look at the change proposal. If your team allows/encourages this kind of behavior then I don't want to work there.