Heather Buchel
If I get to 4k followers on Mastodon I'll tell you my #1 engineering tip that will make you stand out from every other engineer.
Oct 31, 2024 at 6:19pmWeb
Show threadHeather BuchelOct 31, 2024

Just kidding, here is the tip: Look at it. Look at the thing. Do you know how many engineers I've worked with that send a code review request, and when I look at the actual thing, it's completely broken. Just look at it please. Actual senior engineers. Did you load it locally? Did you load the web page? Please, please look at the thing before you send it.

Also, help people out, when THEY send a code review, YOU be the one that looks at it.

Show threadHeather BuchelOct 31, 2024
how many of you thought I was doing a linkedin on mastodon tee hee
Show threadeyrea 🇨🇦Oct 31, 2024
@hbuchel I admit it gave me pause.
Show threadJenniferplusplusOct 31, 2024
@hbuchel oh no, i never thought that. I thought it was 50/50 that the tip would be "it depends" as a joke or "learn to write well"
Show threadnil  @ FURGOct 31, 2024
@hbuchel the times i've had to review code that doesn't run, or doesn't even parse... but at the same time i'm grateful for the respect in my team, since we all forget to run tests locally sometimes
Show threadKen BruckerOct 31, 2024
@nil @hbuchel sometimes can be forgiven. But when I see a pipeline history that is failure after failure… fast path to losing my respect.
Show threadJochie 👨🏻‍💻🏳️‍🌈Oct 31, 2024
@hbuchel so true; I’ve lost count of the number of PRs I’ve seen that have trivial white space issues, obvious typos in variable names, unnecessary changes (someone adding a newline in a part of the code they didn’t touch at all), etc, that suggest to me that they didn’t look at their “giit diff” output before making their commit. 😕
Show threadRaven667Oct 31, 2024
@jochie @hbuchel lol that developer is me. I've had a few busted scripts because my mouse slipped and pasted some random string in the middle of code elsewhere, or missing a quote or semicolon when making a "trivial" bug fix. I too would like to catch those mistakes before they hit prod, but staying on task after it's "done" is harder than I'd like.
Show threadJochie 👨🏻‍💻🏳️‍🌈Oct 31, 2024

@raven667 @hbuchel Something about “Admitting you have a problem is the first step”. 😂

This is where (git or similar) pre-commit hooks are your friend; There you can run the appropriate linters/syntax-checkers, and alert yourself to these kinds of problems. Several of them may be pickier than necessary for “oh shit, I did what” purposes but they can often be instructed to only complain about outright errors and maybe leave out problems with "style" choices.

Also: Unit tests!

Good luck😁

Show threadRaven667Nov 1, 2024
@jochie thanks! I have a colleague tasked with investigating pre-commit hooks, but I'm a little disappointed there doesn't seem to be a way to distribute them when cloning a repo. I understand the security concerns with hidden code running, but it's very inconvenient
Show threadJochie 👨🏻‍💻🏳️‍🌈Nov 1, 2024
@raven667 Yeah, I’ve seen “run this after cloning” instructions in repositories in the past, for that exact reason. Another approach that helps is PR-triggered tests/builds (think GH workflows, for instance) that at least can warn you before you approve/merge the PR.
Show threadmtjmOct 31, 2024

@hbuchel For me it's much easier to run the code when reviewing a PR than deal with the bug after it's merged. And at least two people get to understand the issue if it's raised during code review.

But I don't think of it when splitting the work into tasks, so often there is a complex PR where nothing can usefully run.

Show threadJeff GriggOct 31, 2024

@mtjm @hbuchel

Unfortunate, to split the work that way.

If needed, then we need to remember to run the completed "integrated" code, after all the necessary tasks and merges are done.

Show threadnkOct 31, 2024
@hbuchel i call this a “blindfix” and they barely barely ever work out
Show threadKen BruckerOct 31, 2024
@hbuchel one guy in particular that I work with is that guy. It’s a rare change from him that I don’t have a large number of comments ranging from style, poor implementation to full on brokenness. And he debugs compiler errors in the pipeline. WTF? Build it locally nimrod.
Show threadAurani Casts FireballOct 31, 2024
@hbuchel nothing stands out more than a tool that was clearly never used by the person that made it.
Show threadPhilipOct 31, 2024
@hbuchel Guilty 😣
Show thread🔹 Jakob GivoniOct 31, 2024
Seriously, I can't believe I'm reading any of this in 2024.... 😳😳😳
Show threadJeff GriggOct 31, 2024

@hbuchel

As a professional, I won't create the Pull Request until I have good reason to believe that my changes are ready to go into production.

Others can review and comment if they want. I'm OK with that.

Show threadJeff GriggOct 31, 2024

@hbuchel

As a "reviewer," …

Well …
"Standing out" in a dysfunctional organization can be a problem.

I've been subjected to years and multiple managers doing micro-management where every hour of every day must be reported against manager assigned tasks in Jira. And no, we are not allowed to add "code review" tasks, because it's "not specified by the methodology, and "it would be scope creep."

…

Show threadJeff GriggOct 31, 2024

@hbuchel

So at some point when project management is enforcing that ZERO TIME is allowed to actually *do* code review, I have to say that it starts to be reasonable to spend that amount of time doing it.

And to look for employment elsewhere.

There's a lot of corporate dysfunction out there. 😢

Show threadFred MoyerNov 1, 2024

@hbuchel @genehack ”I don’t like your brace curling”

“Shit works though”

Show threadEndlessMasonNov 1, 2024

@hbuchel
But where does it stop? What? Am i supposed to run the unit tests? Understand what the dev is trying to achieve? Am i supposed to read the documentation they write too?

Outrageous!

Show threadSeanNov 2, 2024
@hbuchel sometimes it feels like the bar is so low. I've been assigned to projects and pull down the code base and realize that it doesn't compile, that it has never compiled, that the last person would just deploy from their own machine and version control was just an afterthought