A Freelance DevOps doing container stuff and automating unhealthy amounts of software.
Need something automated or containerized? Feel free to hit me up :)
Currently I work on a project with someone that absolutely loves to bulldoze comments as per Rule #3.
It's infuriating to spend an hour to actually review PRs, give feedback and discuss questionable additions, only to see that 4 out of 5 reviewers get skipped and the PR merged.
I don't even know how to discuss this with the dev, without it sounding like an attack. Do you have any advice how to go about this?
Depending on your tolerance for confrontation, this scenario can indeed be tricky. I know how I'd deal with it, but most people aren't as comfortable as me when it comes to pissing people off.
The first question I have is: Are you simply leaving "comments"? Or are the comments entered after you've clicked "Start a review"? I'm asking because if you "Start a review", the comments will display as needing to be resolved. But if you haven't started a review, your comments are more like side notes.
Second, what are the PR settings for the repo? Specifically: how many approvers are required before it can be merged? And who has permissions to merge? If the PRs require more approvers, then it's less likely that it gets all the required approvals without your comments being resolved. Similarly, if there are only a few people who can merge, and those people are at least semi-professional, then it's far less likely that unresolved issues get merged. But if the policy is that anyone can merge once it's been approved, then it's far more likely that the dev can find someone to slap an approval on it - and then he can just merge it himself.
Obviously, changing the PR/approval/merge policies requires some buy-in from the rest of the team and it can lead to further headaches. For small teams, it can be impractical to require any more than a single approval. And locking down merge rights can be problematic if, say, only one person can merge - but that person's not in the office at the moment.
But these approaches are all tactical ways to control what is really a systemic issue in your team. Ultimately, the team's leadership (and, by extension, the entire team) shouldn't tolerate that crap. It's one thing if you put a single, small comment on a PR and it gets merged without resolution. That can happen anywhere - usually by mistake. It's another thing if your feedback is being completely disregarded.
If you're not comfortable talking to the dev about this directly, you should definitely be comfortable talking to the manager/lead about it in private. (And if you're not comfortable talking to the manager/lead in private, then you really need to question if you're in the "right place" for you.) Hopefully, the lead would understand your concerns and then communicate them back to the whole team as general policy (i.e., communicate them without having to call you out specifically).
If none of that works, then I ultimately believe that you need to protect your own sanity by simply withdrawing from that dev's reviews. In other words: Don't disapprove his PRs. Don't delete them. Don't comment on them. Don't do anything with them. If you know that your time spent reviewing them will be WASTED, then why continue to spend the time??
I understand that this approach is kinda passive-aggressive and may still lead to some kinda low-level "confrontation". At some point, Jerk Dev will say, "Hey, I submitted this PR. Can you please review it?" And you'll need to say something like, "No - my comments are always ignored on your PRs, so I can't afford to waste the time." But if the team/lead/management won't enforce a general policy of resolving comments, this may be your only real option.
A Freelance DevOps doing container stuff and automating unhealthy amounts of software.
Need something automated or containerized? Feel free to hit me up :)
Depending on your tolerance for confrontation, this scenario can indeed be tricky. I know how I'd deal with it, but most people aren't as comfortable as me when it comes to pissing people off.
I am usually a very direct person. This is very uncomfortable for others, but sometimes I do think to get my point across it's necessary to skip the sugarcoating. Though I have been told to "dial it back a little", which I am failing most of the time.
The first question I have is: Are you simply leaving "comments"? Or are the comments entered after you've clicked "Start a review"? I'm asking because if you "Start a review", the comments will display as needing to be resolved. But if you haven't started a review, your comments are more like side notes.
Currently, we are using bitbucket, which IMHO, is heavily lacking in that regard. It seems the moment you are assigned as an approver, the review has officially started, but can, at any point, be overwritten by the one that created the repo. In my case, that's Mr. Merge-Happy. It might also be some very weird repo settings, which I can't view as a contributor.
What might be important information is, that I explicitly marked the PR as "needs amendment", or whatever bitbuckets English translation for the yellow "not good enough yet" button actually is.
locking down merge rights can be problematic if, say, only one person can merge - but that person's not in the office at the moment.
To me, it looks like this is the case at the moment.
Unfortunately, there is no real "lead" in this project, the higher-ups decided "let's change it up for once! You have a very flat hierarchy, you duke it out amongst yourselves." I'm not sure what else to expect but pure anarchy from this approach. If there is anyone we could consider "lead", it would probably be the dev in question. Which makes this a rather spicy topic for a talk with the manager.
I did have a talk with the dev, because it ticked me off so bad, the response was basically "if the PR is still open after 3 days, I'll just merge it. Rule of cool."
So, as much as I would like to make use of some of the great points you made, I think with that kind of attitude, pretty much every discussion would be a waste of time.
That leads me to the conclusion that the passive-aggressive approach might be the only valid way, if my monologue about "collaboration means working together, not against each other" shows no effect in the near future.
Thankfully, as an external contractor, I can just go on vacation for a few weeks and watch the mayhem unfold from the outside.
Your feedback is very much appreciated, I learned a lot from your response!
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Currently I work on a project with someone that absolutely loves to bulldoze comments as per Rule #3.
It's infuriating to spend an hour to actually review PRs, give feedback and discuss questionable additions, only to see that 4 out of 5 reviewers get skipped and the PR merged.
I don't even know how to discuss this with the dev, without it sounding like an attack. Do you have any advice how to go about this?
Depending on your tolerance for confrontation, this scenario can indeed be tricky. I know how I'd deal with it, but most people aren't as comfortable as me when it comes to pissing people off.
The first question I have is: Are you simply leaving "comments"? Or are the comments entered after you've clicked "Start a review"? I'm asking because if you "Start a review", the comments will display as needing to be resolved. But if you haven't started a review, your comments are more like side notes.
Second, what are the PR settings for the repo? Specifically: how many approvers are required before it can be merged? And who has permissions to merge? If the PRs require more approvers, then it's less likely that it gets all the required approvals without your comments being resolved. Similarly, if there are only a few people who can merge, and those people are at least semi-professional, then it's far less likely that unresolved issues get merged. But if the policy is that anyone can merge once it's been approved, then it's far more likely that the dev can find someone to slap an approval on it - and then he can just merge it himself.
Obviously, changing the PR/approval/merge policies requires some buy-in from the rest of the team and it can lead to further headaches. For small teams, it can be impractical to require any more than a single approval. And locking down merge rights can be problematic if, say, only one person can merge - but that person's not in the office at the moment.
But these approaches are all tactical ways to control what is really a systemic issue in your team. Ultimately, the team's leadership (and, by extension, the entire team) shouldn't tolerate that crap. It's one thing if you put a single, small comment on a PR and it gets merged without resolution. That can happen anywhere - usually by mistake. It's another thing if your feedback is being completely disregarded.
If you're not comfortable talking to the dev about this directly, you should definitely be comfortable talking to the manager/lead about it in private. (And if you're not comfortable talking to the manager/lead in private, then you really need to question if you're in the "right place" for you.) Hopefully, the lead would understand your concerns and then communicate them back to the whole team as general policy (i.e., communicate them without having to call you out specifically).
If none of that works, then I ultimately believe that you need to protect your own sanity by simply withdrawing from that dev's reviews. In other words: Don't disapprove his PRs. Don't delete them. Don't comment on them. Don't do anything with them. If you know that your time spent reviewing them will be WASTED, then why continue to spend the time??
I understand that this approach is kinda passive-aggressive and may still lead to some kinda low-level "confrontation". At some point, Jerk Dev will say, "Hey, I submitted this PR. Can you please review it?" And you'll need to say something like, "No - my comments are always ignored on your PRs, so I can't afford to waste the time." But if the team/lead/management won't enforce a general policy of resolving comments, this may be your only real option.
First off, thank you for the detailed response!
I am usually a very direct person. This is very uncomfortable for others, but sometimes I do think to get my point across it's necessary to skip the sugarcoating. Though I have been told to "dial it back a little", which I am failing most of the time.
Currently, we are using bitbucket, which IMHO, is heavily lacking in that regard. It seems the moment you are assigned as an approver, the review has officially started, but can, at any point, be overwritten by the one that created the repo. In my case, that's Mr. Merge-Happy. It might also be some very weird repo settings, which I can't view as a contributor.
What might be important information is, that I explicitly marked the PR as "needs amendment", or whatever bitbuckets English translation for the yellow "not good enough yet" button actually is.
To me, it looks like this is the case at the moment.
Unfortunately, there is no real "lead" in this project, the higher-ups decided "let's change it up for once! You have a very flat hierarchy, you duke it out amongst yourselves." I'm not sure what else to expect but pure anarchy from this approach. If there is anyone we could consider "lead", it would probably be the dev in question. Which makes this a rather spicy topic for a talk with the manager.
I did have a talk with the dev, because it ticked me off so bad, the response was basically "if the PR is still open after 3 days, I'll just merge it. Rule of cool."
So, as much as I would like to make use of some of the great points you made, I think with that kind of attitude, pretty much every discussion would be a waste of time.
That leads me to the conclusion that the passive-aggressive approach might be the only valid way, if my monologue about "collaboration means working together, not against each other" shows no effect in the near future.
Thankfully, as an external contractor, I can just go on vacation for a few weeks and watch the mayhem unfold from the outside.
Your feedback is very much appreciated, I learned a lot from your response!