I find a lot of times there's push back from developers who don't TDD, and they definitely don't do incremental test and commit.
I find tidying to be entirely about economics. If code is not getting in the way of my work I just don't touch it. Getting in the way could be where I need to understand it to make another change, in which case I may refactor it. It could be where it's core to what I'm changing, and it's difficult to read, inflexible, or whatever, and that too gets in my way. But if it ain't getting in my way, I usually leave it alone. I know that sometimes people like to tidy just because something looks bad, and I've seen projects take a very long time because of that. Code *always* looks bad, and you can actually over do it. :D
This has been my approach as well. When the code impacts my productivity by being unclear or kludgy, then I will tidy it in-place. But, to your second point, I will admit that this is also something I struggle with - sometimes it's difficult to stop tidying because the temptation is too strong to "refactor all the things" - especially on a code base that is heavily laden with tech-debt. Tidying it may have a good long term ROI, but the short-term cost is too high. I put a "Done is better than perfect" poster on my wall to remind me of this...
I've found this to be the most difficult part about tidying/refactoring. One thing I try to do, and have encouraged teams I've worked with to do, is to focus on the code immediately surrounding the change I'm planning to implement. Rather than tidying the whole class or helper methods/functions, just tidy the methods/functions I am actively working in. But this does not always work as some structural changes cause a cascade of changes often unintended. I am eager to learn your recommendation for answering these questions.
First, their feelings are their responsibility and I can’t control them. It helps to appreciate their perspective while also holding your own position—I’m looking forward to the day this is a separate service but I need to make these changes today and tidying will help me.
That's an awesome viewpoint, I can't agree more. It can be tough to deal with people who choose to believe others are evil are stupid. But I think you nailed that one! :D
Be respectful, and kind, and if they still want to "choose" to believe the way they do, there's nothing we can do about that.
“Don’t tidy that. We’re about to replace it.”, “Don’t tidy that. It won’t make any difference.”
Why were you looking in that code in the first place?
I will assume there was a good reason, that you needed to understand something in there, or that you needed to modify it, not that you are just trying to "clean something somewhere".
Here one point I see, is even if it will have no impact to the outside, the customers, the product, may be thrown away tomorrow. It will however have an impact on myself who is currently working on it (and maybe my colleague wo will need to look at it tomorrow).
I try to clean my workplace before changing something and sometime clean my kitchen before cooking.
I use tidying as a tool to better understand a big chunk of code. The process of understanding, building small blocks I can abstract in my brain, is a similar step that I am doing when tidying up. The rest is just some keystrokes to move lines around.
I see the TF? process also as a step to make recognized that your (and team) well being matters.
If we take the rule separate PRs for features and structural changes, then I imagine your story could be 3 pull requests: the formatting change, the new thing, the tidying.
In this case, here it would make sense to reject it as it should be a separate pull request.
But that would mean you move the formatting change to a separate one too.
I've tried that myself, but found that some devs were upset if I do more PRs. They call it noisy, even though I'm literally making it simpler for them. e.g. One refactoring makes it very clear what the intention of the change was. 20 refactorings makes it quite unclear, but for some reason they'd rather it be one PR. :(
I haven't read that particular article before, but that's got a really nice layout. I am doing the TC part of it, but I couldn't get used to the revert part. I also haven't been able to experiment with it with others, as people don't collaborate where I work. *sad face*. But TC is super effective. With covid-19 my productivity dropped by at least 3x, due to network latency over my tiny 5Mbps connection (small town) and when I adopted TC it went up by about 3x-5x.
I haven't tried TCR in "real" work, but I had luck with something going in that direction: being very liberal with reverting whenever I feel like I'm in a mess. I got inspired initially by Corey Haines's https://articles.coreyhaines.com/posts/short-lived-branches
Also, I think another reason I can't get used to the R part of it is that I work on legacy code exclusively, with bad cohesion and high coupling. This usually means large amounts of test setup, so losing that is a pain. At least until that piece of code gets decoupled properly that is. Although the Limited WIP plugin for IDEA allows you to exclude tests, I figured what's the point of doing the 'R' when I literally changed only one line. :D
In this project, a combination of automatic formatters, linters, and git hooks (e.g., prettier+husky for JavaScript) are used, so formatting changes of adjacent code are smashed within a single commit, and by extension, a single pull request.
But this is a technical nuisance; I think the problem is social in nature.
I imagine that there are some cultural prerequisites for TF? to be introduced (I doubt that my colleague B would be enthusiastic, for example); or maybe TF? helps to change the culture? I don't know; let's see what Kent will write.
I find a lot of times there's push back from developers who don't TDD, and they definitely don't do incremental test and commit.
I find tidying to be entirely about economics. If code is not getting in the way of my work I just don't touch it. Getting in the way could be where I need to understand it to make another change, in which case I may refactor it. It could be where it's core to what I'm changing, and it's difficult to read, inflexible, or whatever, and that too gets in my way. But if it ain't getting in my way, I usually leave it alone. I know that sometimes people like to tidy just because something looks bad, and I've seen projects take a very long time because of that. Code *always* looks bad, and you can actually over do it. :D
This has been my approach as well. When the code impacts my productivity by being unclear or kludgy, then I will tidy it in-place. But, to your second point, I will admit that this is also something I struggle with - sometimes it's difficult to stop tidying because the temptation is too strong to "refactor all the things" - especially on a code base that is heavily laden with tech-debt. Tidying it may have a good long term ROI, but the short-term cost is too high. I put a "Done is better than perfect" poster on my wall to remind me of this...
This is precisely the topic I intend to address in the second section of the book.
How do you know when to start? How do you know when to stop? How do you slice the work?
I've found this to be the most difficult part about tidying/refactoring. One thing I try to do, and have encouraged teams I've worked with to do, is to focus on the code immediately surrounding the change I'm planning to implement. Rather than tidying the whole class or helper methods/functions, just tidy the methods/functions I am actively working in. But this does not always work as some structural changes cause a cascade of changes often unintended. I am eager to learn your recommendation for answering these questions.
This will not work well. When you replace the code, you will need the experience from the previous refactoring. That will make a difference.
What are the strategies to also leave the opponent with a feeling that we are not evil or stupid?
First, their feelings are their responsibility and I can’t control them. It helps to appreciate their perspective while also holding your own position—I’m looking forward to the day this is a separate service but I need to make these changes today and tidying will help me.
That's an awesome viewpoint, I can't agree more. It can be tough to deal with people who choose to believe others are evil are stupid. But I think you nailed that one! :D
Be respectful, and kind, and if they still want to "choose" to believe the way they do, there's nothing we can do about that.
“Don’t tidy that. We’re about to replace it.”, “Don’t tidy that. It won’t make any difference.”
Why were you looking in that code in the first place?
I will assume there was a good reason, that you needed to understand something in there, or that you needed to modify it, not that you are just trying to "clean something somewhere".
Here one point I see, is even if it will have no impact to the outside, the customers, the product, may be thrown away tomorrow. It will however have an impact on myself who is currently working on it (and maybe my colleague wo will need to look at it tomorrow).
I try to clean my workplace before changing something and sometime clean my kitchen before cooking.
I use tidying as a tool to better understand a big chunk of code. The process of understanding, building small blocks I can abstract in my brain, is a similar step that I am doing when tidying up. The rest is just some keystrokes to move lines around.
I see the TF? process also as a step to make recognized that your (and team) well being matters.
I've seen something related in a discussion under a pull request at work just today:
- A: ...while you're at it, this part could use some tidying.
- B: Oh, this code was already there, it's in the diff just because of a change in formatting (or something).
- A: Well, OK, I guess...
This type of pushback seems to me somewhat harder to reject.
And of course, it doesn't get any easier to reject it next time.
BTW, I can't help to keep reading "TF?" in my mind as "the f***?" - but maybe this is by design?
Maybe
Hahahah, I read it as "the f***?"as well
If we take the rule separate PRs for features and structural changes, then I imagine your story could be 3 pull requests: the formatting change, the new thing, the tidying.
In this case, here it would make sense to reject it as it should be a separate pull request.
But that would mean you move the formatting change to a separate one too.
Or you separate the PR. That would align authority and responsibility.
I've tried that myself, but found that some devs were upset if I do more PRs. They call it noisy, even though I'm literally making it simpler for them. e.g. One refactoring makes it very clear what the intention of the change was. 20 refactorings makes it quite unclear, but for some reason they'd rather it be one PR. :(
I think this is where the blocking nature of pull requests is particularly problematic. Check out https://increment.com/testing/testing-the-boundaries-of-collaboration/
I haven't read that particular article before, but that's got a really nice layout. I am doing the TC part of it, but I couldn't get used to the revert part. I also haven't been able to experiment with it with others, as people don't collaborate where I work. *sad face*. But TC is super effective. With covid-19 my productivity dropped by at least 3x, due to network latency over my tiny 5Mbps connection (small town) and when I adopted TC it went up by about 3x-5x.
I haven't tried TCR in "real" work, but I had luck with something going in that direction: being very liberal with reverting whenever I feel like I'm in a mess. I got inspired initially by Corey Haines's https://articles.coreyhaines.com/posts/short-lived-branches
Also, I think another reason I can't get used to the R part of it is that I work on legacy code exclusively, with bad cohesion and high coupling. This usually means large amounts of test setup, so losing that is a pain. At least until that piece of code gets decoupled properly that is. Although the Limited WIP plugin for IDEA allows you to exclude tests, I figured what's the point of doing the 'R' when I literally changed only one line. :D
I understand your "you" here as doing the split yourself instead of asking your colleague. I like it.
Yeah, I think this is the best approach, because it brings no extra work to the ones doing code review.
In this project, a combination of automatic formatters, linters, and git hooks (e.g., prettier+husky for JavaScript) are used, so formatting changes of adjacent code are smashed within a single commit, and by extension, a single pull request.
But this is a technical nuisance; I think the problem is social in nature.
I imagine that there are some cultural prerequisites for TF? to be introduced (I doubt that my colleague B would be enthusiastic, for example); or maybe TF? helps to change the culture? I don't know; let's see what Kent will write.