2

Long story short: An older new(ish) starter with more industry experience is blindly ignoring our coding standards. I don't know the best way to resolve this.

Sorry for the long post, thanks for reading!

A new(ish, only a couple of months in) developer at my company who is 10 years my senior in age and industry was tasked with a new project from his start date. This was meant to be almost exclusively his project with help here and there from other members of the team. So for the most part I've not been looking at this partly due to the fact the higher ups wanted it to be a pretty much solo project, (a fresh start), and partly due to other commitments.

I am the most senior developer on the team right now and have been here around 3.5 years, and before he started I shared with him our teams coding standards document. I noted we could discuss any issues with this and look at changing potentially anything if it was in agreement.

He did not start this new project straight away, but was given a couple of small tasks to get used to our environment. I immediately noticed small, but numerous bits of code that we're not following our standards and I chatted with him about this to see if he could be more careful about following it. To which I got a seemingly positive response somewhere along the lines of, "sorry, yes of course. I just need to get used to the change". All sounded fine.

A couple of weeks later when I had a brief meeting to go over his work on the new project I noticed the same issues, and once again mentioned it to him. This time I said we could have a different standard for this one project if he could write up a document explaining it. However I got the same response as the first instance, and he said he would change the code to match our standard.

Now a month or two on we've had one other developer working with him doing the front end work, and I'm now noticing problems occurring. This other developer has no idea how the code works. The fact that it is not following our standards is starting to have a negative effect on our productivity.

The new project has been built on top of a previous application this new developer had built in the past. We all knew this from the start and have accepted there are going to be a lot of differences to what we're used to. This project is requiring a quick turn around, so this decision was accepted. However some decisions have been made by this new developer that are either completely absent from the coding standards or once again the opposite of the coding standards.

This is really starting to get on my nerves at this stage. I look at his code, and my heart starts to sink and in my head I'm saying to myself "why the hell did he do it this way!" or "why didn't he do it the way we've always done it! What's the point of our standards if they just get ignored!"

I was mid way through writing an email to discuss these things with him (currently on vacation), until I realised I may word my frustration incorrectly and cause other problems as a result.

As this developer is a fair bit older and experience, I can certainly understand he may know better ways to do things, but the things I'm picking up on do not fall into this category, and in one case I know for a fact his change is worse (security wise) than the way we've done it in the past.

I'm not sure of the best way to get my message across to him that he needs to stop doing everything his way. I would never go to a new job and just write every bit of code exactly how I do now. I would do it the way the company are used to doing it, and suggest alternatives if I had any real issues with it. This guy is not doing that. I feel like at every point, he's decided to do things differently and has completely ignored our coding standards.

I'm in a more senior position to this developer in the hierarchy, however I'm not his manager, so I don't feel comfortable telling him directly what to do. His actual boss and mine, is not really the type of person to enforce something like this either as he's not a programmer, he wouldn't know where to start, so I'm not sure if that's a solution.

What's the best way to resolve this?

It's worth noting, I have no personal issue with the developer, I get along with him just fine and have worked on tid bits here and there without any issue.

Improve this question
11
  • Have you come to these code standards together as a group, with everyone buying in? Even new coders will have trouble accepting stuff like this "just because".
    – Garrison Neely
    Aug 17, 2016 at 16:30
  • 9
    Any reason you cant use something like StyleCop (or equivalent for your language) in your CI pipeline to reject incorrectly styled code as a failed build? You can even set it up on a commit hook for your source control, so it doesnt even make it into the repo...
    – user34687
    Aug 17, 2016 at 16:36
  • 6
    Your manager shouldn't need to be a programmer to help reinforce the need to align to company standards. I don't know accounting, but if I'm managing accountants and a Sr. Accountant tells me newly hired Accountant isn't following the company's defined standards, I'm going to have a conversation with the newbie to reiterate the necessity.
    – Chris G
    Aug 17, 2016 at 16:36
  • 3
    Are the standards in question simple formatting issues or design issues?
    – Patricia Shanahan
    Aug 17, 2016 at 16:54
  • No programming lead is a problem. I think you need to go to your manager and explain the problem. If he knows how to apply the coding standard then why not just do it in the first place? If he disagrees with the coding standard then he should propose changes.
    – paparazzo
    Aug 17, 2016 at 17:01

5 Answers 5

Reset to default
14

If you are using a properly defined process for build management, including source control, continuous integration and automared unit testing, then you can handle this quite easily - treat incorrectly styled code as exactly what it is, a bug.

In the .Net world you would use StyleCop (or an equivalent) which either takes the code before it is committed to your source control (a pre-commit hook), or it sits in your CI pipeline, and it examines the code for inconsistencies between the code and the defined style the code must conform to. If the code does not conform to the style, then it is rejected and the build (if you are using it in the CI pipeline) is marked as failed.

Extra credit if the failure triggers blame emails to all other developers....

Improve this answer
13
  • 8
    Extra credit if the failure triggers blame emails to all other developers.... I do not think that is particularly helpful
    – IDrinkandIKnowThings
    Aug 17, 2016 at 16:52
  • 3
    @Chad its incredibly helpful, its a very public notification of build issues done in order to solve exactly these issues. You should never break the build, which means bad code should never be committed to the repository - including styling issues if they are considered not acceptable. Dev teams use many different systems for this team-based character building exercise, including blame emails, klaxons on failed builds, electronic notice boards with build status that flash red etc etc etc. It solves these problems very quickly.
    – user34687
    Aug 17, 2016 at 16:56
  • 9
    Public shaming on breaking a CI build (which has at its entire purpose finding failures) is a pretty bad idea. Particularly if you are adding a new functionality to the CI system itself.
    – enderland
    Aug 17, 2016 at 18:18
  • 1
    I have worked in many teams where such emails would end up causing real problems. It might work on your team but most the of the teams I have worked on would have caused far more problems than they avoid.
    – IDrinkandIKnowThings
    Aug 17, 2016 at 20:29
  • 2
    I think sending out a "blame" notice is generally a bad idea. It should just reject the changes without any public notice. If the person can't correct his code to the format needed, then his work would be late and he'd have to come up with a better excuse than, "the source control engine rejected my changes."
    – Dan
    Aug 18, 2016 at 19:29
2

Do regular code reviews with the entire team, either daily or weekly, depending on what's right for your group. Correct any code it before it merges to the main (master?) branch of your repo. It's likely you're just having issues with a dev who can't do the work to spec without plummeting in productivity. Code reviews will help change that, and force all code through a filter that correct errors.

Improve this answer
1
  • 1
    Could you possibly expand this into an answer that explains why this will help and how the OP should go about organising them
    – IDrinkandIKnowThings
    Aug 17, 2016 at 16:42
2

There are two possibilities, and you have to figure out which one it is. Either you have coding standards that are just there to have a rigid standard, and don't actually add any value. Or worse, are contradicting industry standards. And this developer ignores your standards because they don't contribute anything of value. That would be your problem.

Or you have a developer who is so rigid in his ways that he cannot adapt, and that's his problem. (And your problem as well unfortunately). Find out which one it is.

Personally, I'm old enough that I don't care what standards you have as long as you don't have more than one set of standards :-) Most of the time it's like driving on the left or the right side of the street: Each is equally fine, but you better agree with the other drivers. If there are things in your standards that are badly counter productive, I'll tell you, and it's up to you to change it or not. Other than that, code should follow one standard so that's what I do.

It should be enough to ask him politely to adapt his style to the common standard. And everything after that depends on how he reacts to being asked politely. Obviously you should code reviews, and a code review for a week worth of changes that just says "could you please change this to our coding standards and then I review it again" will probably work wonders.

PS. Anyone who calls a style checking tool "StyleCop" seems to have a very bad attitude to people doing their job.

Improve this answer
6
  • 4
    StyleCop is the official name of actual tool created and published by Microsoft (now under the Micorsoft Public License) for checking code style and consistency. It's not a cheeky name for a generic process. stylecop.codeplex.com
    – alroc
    Aug 17, 2016 at 18:41
  • 1
    I ddn't say it was cheeky. I never thought it was cheeky. I said it shows a bad attitude to people. I'm not surprised something like that would come from Microsoft. Process over people.
    – gnasher729
    Aug 17, 2016 at 20:48
  • 1
    Could be a traffic cop rather than an arresting officer, so I think objections the name say more about the objector than about the name.... And I can't believe Im defending a Microsoft product.
    – keshlam
    Aug 18, 2016 at 0:05
  • I'm sure there are some standards we have that are a bit over zealous. I am perfectly willing to change these though. I believe this dev is also willing to change, but he is potentially as you suggest in your second para, to rigid in his ways.
    – not a manager type
    Aug 18, 2016 at 9:47
  • 1
    @gnasher, If that's the association you have with the word, fine; it isn't mine and gods only know what's typical where or for who. Your milage may vary; void where prohibited.
    – keshlam
    Aug 18, 2016 at 14:52
2

There are a couple things your team can do to fix issues such as this.

  1. Add code review as a step in your process for all your projects, and ensure that specific sets of developers/testers/managers are included as reviewers on the pull request. When he submits a pull request that fails to meet coding standards, comment on it and either decline or mark it as "needs work". This works both ways - all your codebases will improve in quality.
  2. You can create a ticket on whatever issue system you use to address the issues, and assign it to him. The ticket would not be closed until your team agrees that it's up to snuff
  3. Schedule a meeting with him specifically designed to discuss these issues. You should both walk away from the meeting with action items such as the above mentioned ticket being prioritized, or a managerial review of coding standards if he brings up excellent reasons for doing things his way

If all fails and he just keeps ignoring you without even trying to address the issue, bring it up with your manager during an appropriate 1 on 1 meeting. If he manages both of you then he can help get to a resolution directly, or can reach out to your coworker's manager to help figure out the situation.

Improve this answer
1

There is a common system called buddy check, which is a form of peer review. On a check-in, another dev - i.e. a buddy - looks at the code and points out small fixes to be done before the check-in*. If you do check-ins every 15 minutes, do the buddy check on every merge. There should be no more than a handful of buddy checks per developer per day.

The buddy must be stated on the check-in comment, which can be enforced with tools. If the person who programmed the code is not available, the buddy is responsible for any questions or issues regarding the checked-in code.

Hold the buddy responsible for code that doesn't follow the standard. If code that violates the standard still gets submitted, then 2 people think the submitted code warrants breaking the coding standard, in which is a sign that the coding standard may need improvement (e.g. it might be too strict).

*In reality, the dev who explains their code to the buddy is usually the one who finds and fixes more issues during the buddy check.

Improve this answer
1
  • gotta love the hit and run downvoters
    – Peter
    Aug 17, 2016 at 17:13

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .