|
Blinkz0rz posted:Miss me with that. Folks can have different ways of approaching a shared vision without the vision implying homogeneity. Diversity of thought is crucial and it's absolutely important to include and accept the loud, opinionated person as part of the process. It's also crucial to ensure that they don't drown out everyone else. Okay. How? Can you give an example? You didn’t answer my previous question about this either (maybe you didn’t see it though). It’s no use to say “well the nitpicker is a white dude so he’s bad”, that’s just an opinion which gets us further away from the apparent goal of accepting him. I really want to know what “accepting” the adversarial (i.e. enjoys giving and receiving tough critiques) teammate into an existing non-adversarial team concretely looks like. I’m finding it very hard to imagine a way to do it where neither side feels criticised, devalued or excluded. e: vv right, the “any color of car so long as it’s black” team. All personalities welcome! As long as you keep your personality within the bounds of these guidelines! Prism Mirror Lens fucked around with this message at 18:58 on Apr 5, 2021 |
#
?
Apr 5, 2021 16:26
|
|
|
|
| # ? May 9, 2024 11:12 |
|
|
Managing a team of wildly different personalities such that everyone communicates productively is a hard problem. I imagine it would take mediation in the short term and setting some team expectations and ground rules for communication in the long term.
|
|
#
?
Apr 5, 2021 16:48
|
|
|
Jose Valasquez posted:Are you this aggro when dealing with your team? You're the only one being aggressive in this conversation. Chill out Very aggressively asserting that all teams except for his are toxic is sort of Blinkz0rz's whole gimmick.
|
|
#
?
Apr 5, 2021 19:21
|
|
|
Blinkz0rz posted:Now let's talk about how the "hostile nitpicker" is usually just a white dude Sure, but the person with the exact opposite approach is also usually just a white dude in this industry.
|
|
#
?
Apr 5, 2021 19:55
|
|
|
In a call a little while back with a few of us devs and our manager, someone asked me if I could just quickly review a 1000+ line commit in code I wasn't familiar with, you know, just so he could push it to QA. I said no, I don't have time today to properly code review that, but I can promise it by the end of the week. Other dev makes a stark point out of the fact that code review isn't a serious thing - just checking for typos and stuff. Manager does not weigh in on this. Other dev a week later gave notice to go work for MSFT. I do not understand this industry.
|
|
#
?
Apr 5, 2021 20:03
|
|
|
Cuntpunch posted:In a call a little while back with a few of us devs and our manager, someone asked me if I could just quickly review a 1000+ line commit in code I wasn't familiar with, you know, just so he could push it to QA. Sounds like institutionally broken process that other dev rightly  ed from  Alternate take: interviews don't select for your ability to participate in good process
|
|
#
?
Apr 5, 2021 20:16
|
|
|
this is a vaguely good excuse to shoehorn in a gem of a tweet that came up in an "ask an employee" thread over the weekend https://twitter.com/QuinnyPig/status/1377786299285864448
|
|
#
?
Apr 5, 2021 20:16
|
|
|
ChickenWing posted:Sounds like institutionally broken process that other dev rightly Other dev was the loudest proponent of 'implementing better code reviews' relative to the previous standard, which I walked into, of "rubber stamping"
|
|
#
?
Apr 5, 2021 21:09
|
|
|
thotsky posted:Sure, but the person with the exact opposite approach is also usually just a white dude in this industry. That's true but I'm not sure what you're trying to get at beyond the demographics of the industry. Plorkyeran posted:Very aggressively asserting that all teams except for his are toxic is sort of Blinkz0rz's whole gimmick. Sorry, who are you?
|
|
#
?
Apr 5, 2021 22:03
|
|
|
Excuse me, his gimmick is insisting that his team is uniquely bad fit for remote work. Keep it together!
|
|
#
?
Apr 5, 2021 22:45
|
|
|
Did you know habitual git squashing developers are mostly just white dudes? Makes you think...
|
|
#
?
Apr 5, 2021 23:14
|
|
|
thotsky posted:Did you know habitual git squashing developers are mostly just white dudes? Agreed, I think having all 75 commits with the message "TICKET-000 chagnes refracted" really preserves the intent of the author.
|
|
#
?
Apr 6, 2021 00:09
|
|
|
thotsky posted:Did you know habitual git squashing developers are mostly just white dudes? I'm starting to think white dudes are overrepresented in this industry!
|
|
#
?
Apr 6, 2021 00:12
|
|
|
Pedestrian Xing posted:Agreed, I think having all 75 commits with the message "TICKET-000 chagnes refracted" really preserves the intent of the author. I need you to understand the transition from "refactoring" to "some more refactoring"!
|
|
#
?
Apr 6, 2021 00:17
|
|
|
"work plz" ah, the commonalities between commit messages and performance reviews
|
|
#
?
Apr 6, 2021 04:21
|
|
|
thotsky posted:I need you to understand the transition from "refactoring" to "some more refactoring"! What about, "trying something" followed by "didn't work, trying something else"?
|
|
#
?
Apr 6, 2021 04:30
|
|
|
"triggering pipeline" "re-running pipeline"
|
|
#
?
Apr 6, 2021 08:09
|
|
|
So you add a pre-commit hook that requires properly formatted commit messages and you end up with.... fix(*): fixes
|
|
#
?
Apr 6, 2021 13:02
|
|
|
NOJIRA: WIP
|
|
#
?
Apr 6, 2021 13:19
|
|
|
Sagacity posted:NOJIRA: WIP Tokyo really got it in this one before gamera managed to take him down
|
|
#
?
Apr 6, 2021 13:24
|
|
|
REBASEME REBASEME REBASEME REBASEME [JIRA-123] implement feature Merge branch "cw/jira-123/feature" into "main"
|
|
#
?
Apr 6, 2021 15:06
|
|
|
Some guy at work (who is not my boss so I don't report to him): "Can you help me get all of this data processed for a call with a client in 2 hours?" Also that guy: "Can we have a status call in 15 minutes?" Do you want your data processed or not, dude?
|
|
#
?
Apr 6, 2021 16:43
|
|
|
Protocol7 posted:Some guy at work (who is not my boss so I don't report to him): "Can you help me get all of this data processed for a call with a client in 2 hours?" 'No.' is a complete sentence.
|
|
#
?
Apr 6, 2021 19:54
|
|
|
Drives me crazy when a reviewer gets hung up on some implementation detail and wants to "wouldn't it be better to ___" without pointing out anything actually wrong with the one I already used.
|
|
#
?
Apr 10, 2021 02:54
|
|
|
brand engager posted:Drives me crazy when a reviewer gets hung up on some implementation detail and wants to "wouldn't it be better to ___" without pointing out anything actually wrong with the one I already used. As our junior has gotten more experienced I've had to check myself on this more often when doing code review. Like is my way objectively better, or just a different way that I would've done it.
|
|
#
?
Apr 10, 2021 04:18
|
|
|
prom candy posted:As our junior has gotten more experienced I've had to check myself on this more often when doing code review. Like is my way objectively better, or just a different way that I would've done it. I've gone through something similar the half year or so, with fresh eyes on old problems leading to different (and sometimes better) solutions than I would have come up with. I've got about 7 years of experience in a pretty narrow field, so it was humbling at first. I decided early on to leave my ego at the door during those discussions and just look at the objective facts. It has helped me become a better engineer, and our customer is super happy
|
|
#
?
Apr 10, 2021 11:28
|
|
|
prom candy posted:As our junior has gotten more experienced I've had to check myself on this more often when doing code review. Like is my way objectively better, or just a different way that I would've done it. I think this is my current biggest issue - I really find it hard to find the line between "this is objectively more readable" vs "I just like to write this way". This has been put into harsher light having moved from a team that was very focused on a very common code style to a team with another Sr. that has radically different opinions on clean code/readability than I do.
|
|
#
?
Apr 12, 2021 14:38
|
|
|
I sometimes feel super conflicted because there's stuff that I just really don't like but my coworkers use every time, e.g. one-letter variable names.
|
|
#
?
Apr 12, 2021 16:17
|
|
|
For nitpicky stuff that doesn't significantly affect function or future maintainability, I will sometimes leave a comment like "X might be better here" but approve the PR.
|
|
#
?
Apr 12, 2021 16:44
|
|
|
Pedestrian Xing posted:For nitpicky stuff that doesn't significantly affect function or future maintainability, I will sometimes leave a comment like "X might be better here" but approve the PR. That's what I do too, but it still makes me feel like an rear end. I don't know how people will take it.
|
|
#
?
Apr 12, 2021 17:05
|
|
|
Leaving suggestions has worked well for me. It has lead to good discussions with junior devs who appreciated learning something new. It has also lead to humbling moments when I proposed solutions that others thought were overly complex with little benefit. Junior devs in my team usually make a lot of "nitpicky" mistakes, so at a certain point in the review, I stop pointing out when their bigger review fixes contain these mistakes. I feel that they would otherwise lose motivation when almost every change they make has a mistake. This problem has solved itself over time anyway.
|
|
#
?
Apr 12, 2021 17:19
|
|
|
In my experience, if someone is just making constant small mistakes, they are probably rushing and not even reviewing their own code before opening the PR. Giving a ton of nitpick comments isn’t really addressing the root cause, just wasting the reviewers time.
|
|
#
?
Apr 12, 2021 17:31
|
|
|
smackfu posted:Giving a ton of nitpick comments isn’t really addressing the root cause, just wasting the reviewers time. Agreed, nitpick comments can be really exhausting to deal with since you're putting a lot of responsibility onto the contributor and still consuming as much of your time as is possible in this process for filing them. IMO: Never nitpick, there is either a code style issue or there is not. Highlight the errant code and provide either mention of, or a direct link to, the style guide that mandates an alternative approach. If there is no style guide, I wouldn't waste my time trying to create an implied one in pull request comments, there are better forums for that.
|
|
#
?
Apr 12, 2021 17:58
|
|
|
Where/how do you objectively draw the line between a nitpick and something you feel is going to hurt maintainability / the ability for people to quickly make sense of code 3 years from now?
|
|
#
?
Apr 12, 2021 18:04
|
|
|
What I'm looking to give/receive from code review is not "12 rats says this looks good to him", it's "12 rats certifies that this code meets the standards and guidelines set out in <team documentation> to the best of his knowledge". My "I think this pattern is a bad idea, and the codebase should be structured in this way" feelings go to meetings where we discuss and maintain the style guide. Code review is decoupled from that process so that we can all do it faster and more predictably. If you work in a shop under a heavily compliance burden you will also often find satellite processes orbiting code review, in particular, I used to have to check a box saying "I thought about these changes as they relate to <each letter in STRIDE threat model> and found no issue with them". If I were clogging that process up with comments about, for example, nested for loop vs nested list comprehension, I would be spending too long in code review which is a double offense in that it wastes my own time as well as the time of the contributor. It's much easier to just copy paste "All strings should be double quoted" out of the guide on my pull request comments and then move on, even if I don't necessarily agree with always using double quotes or care one way or the other. 12 rats tied together fucked around with this message at 18:13 on Apr 12, 2021 |
|
#
?
Apr 12, 2021 18:11
|
|
|
12 rats tied together posted:It's much easier to just copy paste "All strings should be double quoted" out of the guide on my pull request comments and then move on, even if I don't necessarily agree with always using double quotes or care one way or the other. Ideally this kind of static analysis / linting should be handled by CI/CD so it's all taken care of before a human has to even look at a PR. Having a robot say nahhhh doesn't hurt anybody's feelings and it's consistent. There's a couple of teams here that have an emoji-code that they use before any PR comments that describe the intent / severity / "tone" of a comment so there's a less ambiguity and you don't have to do a lot of couching. Plus there's an explicit emoji for nitpicking for when you just neeeed to say something but it's not a blocker. (Kinda like this: https://github.com/erikthedeveloper/code-review-emoji-guide)
|
|
#
?
Apr 12, 2021 19:25
|
|
|
JehovahsWetness posted:Ideally this kind of static analysis / linting should be handled by CI/CD so it's all taken care of before a human has to even look at a PR. Having a robot say nahhhh doesn't hurt anybody's feelings and it's consistent. Yeah, that's kind of what I was (obliquely) getting at: style nitpicks are in the linter (fed by configuration generated from documentation). They'll never make it to PR unless the contributor is appending "ignore this" annotations to their code or if they found an incantation that confused the linter, both of which are real issues with concrete and definable paths forward instead of "nitpick, you can ignore this, which means you now have to decide whether or not to ignore this".
|
|
#
?
Apr 12, 2021 19:58
|
|
|
I like nitpicks, as long as they are reasonably constructive and clearly labeled as such. A "hey, if you slightly restructure this, you can mark this whole thing as const" is useful to me. Of course there are not useful nitpicks... "I think this should be called ComputeCosts and not CalculateCosts" is very likely an example of such.
|
|
#
?
Apr 12, 2021 21:04
|
|
|
The obvious answer here it to tailor code review to the team culture, the code standards and the individual, but I think when it comes to junior engineers, give them enough rope to really hang themselves later. The experience of actually trying to maintain unmaintainable code is way more powerful than someone saying, 'this is difficult to maintain'. Ideally a more junior engineer is on a project where scuffed code isn't that big of a deal, as long as it works. Let them make their own design and architectural decisions and let them do a really bad job. When it comes to maintenance and updates, I'd put them on point for it and let them scrub their own work. Actually having to work through your own mess is such a strong motivation to write better code. It also gives you a lot of personal perspective on what 'good code' actually looks like, and more often, what 'bad code' looks like. I don't think it's particularly helpful to say the words to people unless you are willing to explain in detail why a certain pattern is superior. Even then, I think a complete understanding of those ideas is not well attained unless you experience it first hand. This has been how we've trained up new engineers and the results have been very positive - we end up with people that not only know how to better architect systems, but also know why they want to do it a specific way and can speak to it technically in a way that is tempered by real experience. Let your engineers write bad code more.
|
|
#
?
Apr 12, 2021 21:28
|
|
|
|
| # ? May 9, 2024 11:12 |
|
|
We culturally prefix nitpicks with "Nit:" in the comment text and interpret this as "I really don't care if you change this or not, but did want to bring this to your attention in case it was unintentional"
|
|
#
?
Apr 13, 2021 01:50
|
|
