Settings

Theme

Engineering dogmas it's time to retire

newsletter.manager.dev

20 points by kiyanwang 8 days ago · 23 comments

Reader

hakunin 7 days ago

Every once in a while I get an opportunity to share my 4 reasons to leave a code comment:

1. An odd business requirement (share the origin story)

2. It took research (summarize with links)

3. Multiple options were considered (justify decision)

4. Question in a code review (answer in a comment)

Important caveat for number 4: if your code can be restructured in a way that answers the question without a comment, do that instead.

This originally comes from an article[1] I wrote in 2021 titled "Writing Maintainable Code is a Communication Skill".

[1]: https://max.engineer/maintainable-code

  • joshka 7 days ago

    I think there's probably a 5th one that's new-ish. Code isn't where the value is now that agentic tools can whip out a solution to just about anything in no time, so the commentary provides semantic grounding that allows you to navigate generated code easily.

    It's kind of like some of the existing reasons, but there is a difference there.

manmal 7 days ago

Code reviews are not only about raising quality, but mainly about communicating changes to the wider team. Suggesting to eliminate code reviews when LLM use is so rampant is also quite uhm courageous.

  • nick4 7 days ago

    Communicating changes and communicating learning too! Every few years I rewatch one of my favorite videos on code reviews: https://www.youtube.com/watch?v=PJjmw9TRB7s.

    Asking questions on code reviews is one of the most powerful tools to learn more about a codebase, and fostering a culture where junior devs feel empowered to ask questions is one of the best ways to help junior devs succeed.

  • cloogshicer 7 days ago

    The author never suggested to eliminate code reviews entirely. Just to give individuals more autonomy, which is great in my book.

    • manmal 7 days ago

      PRs don’t really hurt autonomy if stacked branches are used routinely. Those do hurt speed, yes, but not autonomy. PRs are so important that I‘d never skip them within a team.

      • cloogshicer 7 days ago

        If you have a policy in place that forces engineers to wait for review before merging each PR, then yes, by definition they have less autonomy. It might still be worth the trade off in your situation, but I like the suggestion in the article where senior devs can decide themselves whether they want their code reviewed or not.

    • heyitsdaad 7 days ago

      Hard no buddy. Junior dev means junior code and junior judgement. Countless times we had prod issues because some dev thought the change was harmless and they didn't need review.

      • cloogshicer 7 days ago

        In the article, they specifically exclude juniors and people who are still being onboarded.

        • jinushaun 7 days ago

          I can’t find that disclaimer in the article.

          • AntonZ234 7 days ago

            OP here. In this one the closest is probably: "I love the process at Pylon: engineers merge their own code and only request reviews if they need input, think they have a risky change, or are still onboarding. "

            But I fully agree that for juniors it makes sense to have it mandatory.

000ooo000 7 days ago

Linear Ad

  • sublinear 7 days ago

    > Switching to another tool felt like a huge project that was just not worth it. Linear took that to heart, and made switching super simple with a 2-way sync, keeping your legacy tool updated. I haven’t met an engineer who tried Linear and didn’t like it. Teams that switch to Linear see 2x more reported issues - engineers actually want to use the tool. More visibility => fewer meetings => happer engineers.

    Yeah that's very clearly an ad. They didn't even try to be subtle lol.

  • tzs 7 days ago

    No, it is an article that includes a short in-article ad for Linear. It's the text equivalent of when a YouTube video thanks some company for sponsoring the video and spends 30 seconds saying some good things about them before resuming whatever the video is about.

    This is good. This is the kind of advertising that people here usually say that sites should be using if they need ads.

    • 000ooo000 7 days ago

      When a video says "so check out NordVPN" before returning to gardening content, I don't have any concerns that the gardening content is influenced by the advertising - they're unrelated. This article is about software development and contains an ad which is irrelevant to everything but software development. That's completely different.

      • tzs 7 days ago

        That's basically the same way magazine ads work. If the magazine focused on a particular category most of the ads would be focused on that category too.

        For example "Chess Life" mostly contains ads that are irrelevant to everything other than chess. "QST", a ham radio magazine, mostly contains ads that are irrelevant outside of ham radio.

        This is what I most often see people here suggesting as the right model for internet advertising.

        • AntonZ234 7 days ago

          Thanks tzs.

          OP here. I actually feel that having ads that are relevant to the people reading are better both ways, as you might actually learn about a good tool :) (I try at least to only work with products I believe in).

          I felt that having the ad between line dividers, and having this: "Thanks Linear for supporting today’s article!"

          should be enough, but maybe I'm mistaken.

          • tzs 7 days ago

            It might be a little better to put the thanks at the top of the ad instead of the bottom.

  • alienbaby 7 days ago

    Exactly what I thought and why I checked the comments , an lo and behold it's not just me..

x3n0ph3n3 7 days ago

Sometimes code reviews and approvals are required due to various conpliance regimes that dictate it as part of the Software Developement Lifecycle (SDLC).

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection