3 ms·

> Do junior developers not take any pride in their work? Yes. I have lost count of the number of PRs that have come to me where the developer added random bla

> Do junior developers not take any pride in their work? Yes. I have lost count of the number of PRs that have come to me where the developer added random blank lines and deleted others from code that was not even in the file they were supposed to be working in. I'm with you -- I review my own PRs just to make sure I didn't inadvertently include something that would make me look sloppy. I smoke test it, I write comments explaining the rationale, etc. But one of my core personality traits (mostly causing me pain, but useful in this instance) is how much I loathe being wrong, especially for silly reasons. Some people are very comfortable with just throwing stuff at the wall to see if it'll stick.

alfons_foobar 10mo ago
> added random blank lines and deleted others from code that was not even in the file they were supposed to be working in. Maybe some kind of auto-formatter?
- rootusrootus 10mo ago
  That is my charitable interpretation, but it's always one or two changes across a module that has hundreds, maybe thousands of lines of code. I'd expect an auto-formatter to be more obvious. In any case, just looking over your own PR briefly before submitting it catches these quickly. The lack of attention to detail is the part I find more frustrating than the actual unnecessary format changes.
  - Aeolun 10mo ago
    Why would you are about blank lines? Sounds like aborted attempts at a change to me. Then realizing you don’t need them. Seeing them in your PR, and figuring they don’t actually do anything to me.
    - baq 10mo ago
      More likely artifact of debug prints being removed.
ok_dad 10mo ago
> Yes. I have lost count of the number of PRs that have come to me where the developer added random blank lines and deleted others from code that was not even in the file they were supposed to be working in. That’s not a great example of lack of care, of you use code formatters then this can happen very easily and be overlooked in a big change. It’s also really low stakes, I’m frankly concerned that you care so much about this that you’d label a dev careless over it. I’d label someone careless who didn’t test every branch of their code and left a nil pointer error or something, but missing formatter changes seems like a very human mistake for someone who was still careful about the actual code they wrote.
- hoten 10mo ago
  I think the point is that a necessary part of being careful is reviewing the diff yourself end-to-end right before sending it out for review. That catches mistakes like these.
  - code_for_monkey 10mo ago
    i myself have been guilty of creating a pr and immediately pushing a commit to clean that stuff up

alfons_foobar 10mo ago

> added random blank lines and deleted others from code that was not even in the file they were supposed to be working in. Maybe some kind of auto-formatter?

rootusrootus 10mo ago
That is my charitable interpretation, but it's always one or two changes across a module that has hundreds, maybe thousands of lines of code. I'd expect an auto-formatter to be more obvious. In any case, just looking over your own PR briefly before submitting it catches these quickly. The lack of attention to detail is the part I find more frustrating than the actual unnecessary format changes.
- Aeolun 10mo ago
  Why would you are about blank lines? Sounds like aborted attempts at a change to me. Then realizing you don’t need them. Seeing them in your PR, and figuring they don’t actually do anything to me.
  - baq 10mo ago
    More likely artifact of debug prints being removed.

ok_dad 10mo ago

> Yes. I have lost count of the number of PRs that have come to me where the developer added random blank lines and deleted others from code that was not even in the file they were supposed to be working in. That’s not a great example of lack of care, of you use code formatters then this can happen very easily and be overlooked in a big change. It’s also really low stakes, I’m frankly concerned that you care so much about this that you’d label a dev careless over it. I’d label someone careless who didn’t test every branch of their code and left a nil pointer error or something, but missing formatter changes seems like a very human mistake for someone who was still careful about the actual code they wrote.

hoten 10mo ago
I think the point is that a necessary part of being careful is reviewing the diff yourself end-to-end right before sending it out for review. That catches mistakes like these.
- code_for_monkey 10mo ago
  i myself have been guilty of creating a pr and immediately pushing a commit to clean that stuff up