Weeknotes -- critiquing my own pull request

This week, I let a PR get too big. Everything in the PR relates to one another, but it could have been broken up into more manageable pieces. I just kept having one little thing to fix! And now, it’s at 400+ lines of code changes across 16 files. Yikes!

Instead of beating myself up about it, I thought I would talk about what makes this PR so unwieldy.

What’s wrong with it?

In short: it does too much.

• It adds several new URLs and changes the behavior of existing URLs across two apps.

• It adds a manager method to a totally different app.

• It adds a new field and overrides some default model behavior.

• It makes minor changes to several template files.

In a code review, all of this is cognitive overhead. For whoever reviews this PR, they have to jump around a lot to check my work. It’s extra work to make sure they are reviewing the right view for the right URL for the right test. It’s extra work to make sure everything has tests. It’s difficult for me to even explain my own changes, because there are so many.

This would work better as several PRs. It would be easier for me to explain my changes, and easier for a reviewer to understand them.

It got this way because I was adding a ManyToMany relationship where one didn’t exist before, and trying to change the existing functionality to accommodate this all in one PR.

When I began, I didn’t think it would get so hairy. But along the way requirements got clarified that added some work, and my own focus was disrupted by household illness and then jury duty. It wasn’t even a case of scope creep. All of this would have needed to be done for the feature to have been considered “complete.” But it didn’t need to be done in one PR.

It was just one of those weeks where I felt behind all week, and by the time things felt clearer, I’d dug myself into a little hole.

The PRs I should have opened

• Adding the new manager method. It’s a perfect little PR — a few lines of code in managers.py, then a test, and you’re good to go.

• Adding the SlugField to a model, and then using the slug in the URL instead of the PK. This could have been two PRs, but I probably would have done it in one. This would have kept a database change and its side effects in their own PR. It’s a very straightforward enhancement, and is perfect for its own PR (or two).

• Adding each new URL and its corresponding view in their own PRs. Each view had to override some default behavior. I think if I had done them to completeness, including tests, one at a time, it would have made the opportunities to make the code DRY-er more apparent. As it was, I was jumping around a lot and I think the code is a bit repetitive.

• Making the changes to the behavior of the existing views in their own PRs, for similar reasons as above. Since I changed existing behavior, having those changes in small PRs makes it very clear what changed.

• Cleaning up the breadcrumbs and links in the templates. I tried to do this as I went, but it involved a lot of context switching. Especially since this project is still in dev, merging in changes that meant breadcrumbs needed some TLC wouldn’t have been the end of the world. And it would have been faster to get the other changes done, so I wouldn’t have had to do so much back-and-forth.

Each of these would be easier for me to explain in a pull request description, and easier for a reviewer to review accurately.

… I might just re-do them.

Learn from my mistakes, friends.

In other news…

• My cat is doing awesome at going in his crate.

• My current professional development project is learning about docs in a more official way. I’ve bought some books, I’m helping a friend revamp the docs for a project he maintains, and I’m planning to write about it! Let’s learn what we’re supposed to be doing with our docs together.

• I’m finishing up another quilt. It’s all sandwiched and basted. I’m planning to free-motion quilt it. It already has a recipient and I’m excited to complete it!

• I’m trying to write more. It’s hard! So I’m going to try to write more about why it’s hard.

• Despite feeling a bit behind all week, it’s been a good week! I feel generally energized at work. We have some new projects coming up that I’m excited about. I think I’m going to get to stretch my wings in new directions a bit this year, and I am ready for it.