3 ms·

> Hey, I ran your solution and it's working nicely. Thanks a lot! By the way, recently I've been working on profiling our code so performance is on the top of m

by pxc 3y ago

> Hey, I ran your solution and it's working nicely. Thanks a lot! By the way, recently I've been working on profiling our code so performance is on the top of my mind. I ran the profiler against this PR and noticed a dip in performance in one of the functions. Do you want to hop on a call sometime to explore a few ways we can refactor xyz function? Really curious to hear what you think and after thinking about this for a while I have an idea or 2. But that's making a PR comment into a meeting, which is something I'd kinda dread and want to avoid. That said, I agree that this > Your method technically works but next time try xyz instead. It's more efficient. ;) is awful. But it's not just the use of an emoticon. It's that it's the wrong emoticon. > this works, but xyz would be better (more efficient) :) Would be a bit ambiguous in tone, but I think at least invites an optimistic reading as well. For me I'd rather the niceties in a PR (as well as proposals to meet) live in a comment at the beginning or end of a PR. Individuak requests for specific changes within a PR review should be clear and simple, not bogged down in formal crap or fluff, imo.

nickjj 3y ago
> But that's making a PR comment into a meeting, which is something I'd kinda dread and want to avoid. Even if it's a new hire that you may not have met yet beyond a hello in Slack?
- pxc 3y ago
  Yeah, for me at least. I feel like that would make a meeting more awkward, not less. Also, if I were in the new hire's shoes, scheduling a meeting that's essentially about criticism would make me more anxious, not less. And the time gap between the scheduling of the meeting and the start of the meeting might be pretty fretful. If someone just lays out what they want changed in 1 or 2 sentences, I can immediately know that it's not some big problem they want to meet about. At the same time, I would feel bad if I found out that I had really distressed a new hire in a code review. And I do try to soften things when I say something that I anticipate might be perceived as harsh in a way that I don't intend. (I use emoji a lot, I DM people, and I tend not to use standalone sentences that end in periods except in email. I use parentheticals and qualifiers, make jokes, etc.) I do schedule meetings to go over things including criticisms/problems/objections, but only when I feel like it would be faster and more natural, or I think it's really essential that I make a realtime demand on someone's attention to get their input.

by pxc 3y ago

nickjj 3y ago

> But that's making a PR comment into a meeting, which is something I'd kinda dread and want to avoid. Even if it's a new hire that you may not have met yet beyond a hello in Slack?

pxc 3y ago
Yeah, for me at least. I feel like that would make a meeting more awkward, not less. Also, if I were in the new hire's shoes, scheduling a meeting that's essentially about criticism would make me more anxious, not less. And the time gap between the scheduling of the meeting and the start of the meeting might be pretty fretful. If someone just lays out what they want changed in 1 or 2 sentences, I can immediately know that it's not some big problem they want to meet about. At the same time, I would feel bad if I found out that I had really distressed a new hire in a code review. And I do try to soften things when I say something that I anticipate might be perceived as harsh in a way that I don't intend. (I use emoji a lot, I DM people, and I tend not to use standalone sentences that end in periods except in email. I use parentheticals and qualifiers, make jokes, etc.) I do schedule meetings to go over things including criticisms/problems/objections, but only when I feel like it would be faster and more natural, or I think it's really essential that I make a realtime demand on someone's attention to get their input.