Director of Eng gave me surprisingly poor feedback. Should I escalate to VP?

2 points by angebracht ↗ HN
I'm a Senior Software Engineer who was called out on the following:

1. Having too many PRs open, closed, and in draft 2. Having too many commits 3. Too many comments from others in my PRs 4. Pushing commented code & code with git artifacts in it

In my defense:

1. Since Q1, my team has reprioritized a lot of work. There hasn't been a discussion about what to do with the work that we all started. Also, I open a draft PR the moment I make my first commit on a branch, for visibility.

2. I commit a lot. Commits are my safety net. I use `git reset` tremendously throughout the day to go back to different states in the code I'm working on.

3. The director expects senior engineers to have very little chatter on their PRs. They should be ready to merge with little discussion. What he didn't see was that most of the comments are from Hound, our linter. Rather than linting before pushing, I enjoy waiting for Hound's comments and fixing them afterward.

4. I made a mistake. I've been forced to comment out code recently in some of our microservices as a temporary fix while working locally. I forgot to uncomment the code before pushing to GitHub. The director went through my commits and saw some commit messages like "uncomment code", "remove commented block", and reprimanded me for that.

I've been an engineer for 10+ years and no one has called me out on any of this. This is how I like to work. I don't see a real problem here.

I can squash my commits, lint locally, and stay blocked when services aren't working on my machine. But I'm mostly concerned about why this director is calling out this bullsh*t.

I spoke to my former manager, and he said it'd just be easier to appease him. But it feels like the director is looking for a reason to let me go by finding incompetencies in my performance. I do not want to work in fear or with paranoia.

Should I have a chat with the director's manager to ask about the feedback and what he suggests I do about it?

7 comments

[ 3.1 ms ] story [ 31.4 ms ] thread
> Rather than linting before pushing, I enjoy waiting for Hound's comments and fixing them afterward.

That's a poor practice, IMO.

A PR should mean that code is ready to merge, so it must have been linted and tested beforehand, so I agree with Director of Eng that there should be little chatter.

This probably goes hand in hand with your practice of opening "draft PR".

Now, if Hound is only accessible to you that way and you think it is a valuable tool to use before opening a PR then this may be a worthwhile discussion to have with him.

yes and this is why a lot of us have started runnning CI on PR's and failing PR's if they don't pass basic quality gateways like linting.
Write down Your issues with his review on a sheet of paper, like You have done here but be as specific as possible.

Take that paper and seal it up in an envelope and don't open it or do anything about your issue for a week. Nothing is going to happen to your job in a week unless You force things.

In a week or so, open up the envelope and read what You wrote, if You still feel the same way then escalate.

Sometimes a bit of time and a sober second takes the sting out of criticism, it is possible You are right but far better to take action when your emotions are in check. It might just be a poorly communicated attempt at constructive criticism from a new manager.

No. Skipping up the chain of command is very seldom a good idea.

Honestly, if this is the type of evaluation a senior engineer is getting, you should find a better place to work.

If you're going to have a genuine conversation about how to improve communication and your relationship with your manager and to gain a better understanding of your working relationship with them, certainly. It's fine and often a good idea to have a chat with your managers manager in the instance it's not about a specific issue, but about a genuine desire to improve the relationship. If you're going to find out "why this director is calling out this bullsh*t", probably not a good idea.
This is legitimate feedback. It's SMART feedback.

If you go to his boss with I don't like this feedback. what is your expected outcome? What would you expect if someone when to your boss to complain about legitimate feedback you gave someone else?

You may not like it but he has laid out exactly how he measures engineers and how he expects his managers to measure them.

If you aren't able to accomodate to meet his measurement and you aren't comfortable knowing you don't meet his expectations, find a team with a different culture.

IMO going to his boss saying I don't feel that this very legitimate feedback will likely only accelerate your departure because it sounds like the director has reciepts to back all his statements. You should make every effort to address it directly with him and your manager before you escalete with no real grounds to escalate.

> I open a draft PR the moment I make my first commit on a branch, for visibility.

If the people responsible for reviewing your work don't want to see your work in progress, just wait to open the PR. Consider that they may be inundated with all kinds of notifications and trying to silence things that are not immediate action items. Don't take it personally. If your director prefers it, just wait to open the PRs.

> I commit a lot.

There is nothing wrong with this. But consider that your commits ought to be easy for others on your team to use. They should be discreet units of work. This is useful for debugging if someone needs to use `git blame` or `git bisect` to figure out why a piece of code does what it does or when and why it was introduced. You can rewrite your commit history into discreet units of work by using `git rebase -i` and squashing some (or all) of the commits in your branch together before you open your PR.

> The director expects senior engineers to have very little chatter on their PRs. They should be ready to merge with little discussion.

In general, communication about your work as an engineer is important. It's one of my pet peeves when team leads complain about too much communication. That being said, there are at least two legitimate reasons for the director's position: first, as I mentioned before, they may have too many messages coming in and be trying to cut back on some unnecessary noise. Help them out with this by avoiding unactionable comments and things that you can take care of yourself.

> Rather than linting before pushing, I enjoy waiting for Hound's comments and fixing them afterward.

I suggest installing local linting tools in your IDE. There's no reason to burden your team with extra messages that you could have taken care of on your own machine.

> The director went through my commits and saw some commit messages like "uncomment code", "remove commented block"

While it's not a good practice to commit commented code, sometimes mistakes happen. You can squash these commits into the previous ones so they don't pollute your git history. Remember, half the point of git commits is to understand how and why your code got to it's current state. If you have to pass a bunch of accidental, cleanup, or whitespace commits to get to the information you are looking for, it can make it really hard to work with the code base in the future.

In general, I don't really see anything wrong with this feedback, though I understand that it can be annoying to have to change your preferred working style. My advice, never escalate something like this. If you can't come to an agreement about it with your director, you have two options, either suck it up and deal with it, or start looking for a new job.