How do you feel about people contributing code to yours? - Developers

So I'd like to put forward this question to the professionals after finding colleagues contributing code, ranging from "styled differently" to "ugly and bug-prone", in systems that I've created. I proposed we should introduce a code style guide, especially seeing as we're a fast growing company and we've hired a lot of new engineers, and ideally code reviews as well. Regardless, I was wondering: If someone contributes code to a system that you feel ownership over, and that code is styled or structured in a way you wouldn't have, are you able to let that go or are you compelled to restructure it?

I usually try to explain why I would've done it differently and if they have any reasonable counterpoints, I would allow it or even change my own methods to include those differences. Other than that, I would be compelled to refactor it or if I don't care much about an old project I would just leave it.

I've not really dealt with this before, but I imagine if you're working in a team then in terms of code styling (e.g. tabs/spaces, curly bracket on new line) then you would have to come up with a guideline, and if you're in ownership then the guideline would be based on your styling, providing you aren't using some sort of code styling which is bizarrely different from the norm.

It really is very different; my previous company was the extreme opposite by making sure every single commit was reviewed beforehand, and I enjoyed that. We also have a lot of developers who are fresh graduates/junior and could use the feedback to learn and fix their mistakes. Unfortunately right now there's no way for them to receive that feedback. I've been collecting examples when I encounter them and making some noise to our management about it. We had a discussion about it but I'm not expecting change any time soon, and he didn't seem to keen on me giving the feedback to the developers directly. I asked the question because I wondered if I had a problem with 'letting go' or if I should push harder for there to be a system of feedback for developers. A set of rules to base the feedback around, e.g. a code style guide, would probably be a good first step.

Do you have some sort of CI pipeline that can knock back code with bad formatting? I feel like consistency can be a bit of hard sell (you can argue that if it works it works) but if you can get permission to enforce some mutually agreed standards that might be a good start.

There's a reason that pull requests exist.

It's called coding standards and there's a reason people use them. So that other contributors can reasonably expect what something will look like and thus work more on code instead of worrying more about what the code is. Benefits of Coding Standards Why You Need Coding Standards It's better to be consistent now than to deal with years of inconsistencies when the problems finally get in the way of any progress. Just take a look at Clockwork, even though there's practically no readily available documentation I can still figure out what I need to know by using ctrl+f and reading what the code actually does. There's thousands of lines and each line more often than not looks very similar to any other line. Reading the code is comfortable and easy, not strenuous and difficult. Here's Clockwork's document on coding standards, it's 3 pages long, half of the document is just code samples: Clockwork Coding Standard Point being that it wouldn't take much to setup a coding standard, the difficulties lie within on-boarding other members in using the standards. Someone doesn't follow the coding standards? Tough luck. Reject their commits 2-3 times stating why and they'll eventually get the message and fix their shit (although every being on the same page and on board with using the coding standard would be preferable).

We also have a lot of developers who are fresh graduates/junior and could use the feedback to learn and fix their mistakes. Unfortunately right now there's no way for them to receive that feedback. So your company hires juniors, put them to work with no oversight and expect things to actually get done properly? Sounds like a road for disaster to me. Also these juniors are never going to improve if they get no feedback on their code. If there is a lead developer/project lead in this company they aren't doing their job. TBH I would be looking for an opportunity to jump ship, though I understand if that is not possible for you at the moment.

Depends on the project. I wrote a silly user highlighting script for another forum, and if someone wanted to contribute to that I'd probably look over their code and adjust it to how I'd want it. (feel free to pick on my code) On a larger project that actually intends to involve other contributors I'd reject it outright for pulling crap like putting the opening brace on a newline.

If it's written in a way that is better than I have, then I'm fine with it. If I see someone changes or adds to my code that's just bad then I'll have a problem with that.

I do that Should I stop doing that?

It's more a question of style, really. Personally I hate it, others disagree and think it makes the code look cleaner. The important thing is to look at the code you're contributing to and do it how its already done.

If somebody wanted to contribute to a project that I owned I'd strictly enforce a style guide as a level-0 rejection cause for commits, It's 0% my job to make sure that other's code fits with the project. Extremely bad opinion, especially for one-line blocks

Consistency is not a hard sell. In my experience, it is harder to sell doing something different than doing something with consistency. Usually when it comes to coding style, there are already standards set in place by the <insert your language here> community. As long as your inline with what the community dictated as "good style" for a certain languae, that is easy to argue because you have the community backing you. Coding style shouldn't be a big deal, and you shouldn't have to create an crazy new coding style for each project, the community likely already has a good guideline for you depending on what language you're using.

I usually just select all, cut, paste.. and it formats it how it should be.

There's more, at least on MonoDevelop: Auto-format on save Shortcut to auto-format (Ctrl-E + D for me) If I had to put code conventions on my projects I'd just make that a link to MS' docs on conventions. Basically, follow the standard guidelines of a language, good for consistency, especially if you intend to make an API.

At work I hate it because it means merge conflicts

To someone technical absolutely. I'm not sure which part of management the original poster discussed it with but unfortunately not all parts of a business understand the need.

What it actually means is a team member constantly fucks around with parts of the project he's not assigned to

You mean resharper, right?

He's the manager, but he's also a programmer, although I'm not sure he understands the need for consistent code, and maybe I haven't communicated it well enough yet. So far I've talked about it with him twice, and I don't feel like I'm being taken seriously. Our projects are quite fast-paced and short, and he's very busy, so I understand it's not his top priority. Regardless I don't believe that if he wasn't busy that he then would take it seriously. We are definitely using git wrong, though, but that's a separate fight. I'll continue to try and introduce some code quality here, for the benefit of myself and the product, but I'm not planning on staying here for long regardless.

Okay, so he's a manager who is technical. Does he produce code for the team? If not, he probably shouldn't be looking at code reviews anyways. I believe a manager shouldn't be making decisions about things like syntax. That's micromanagement in my opinion. They should be able to guide the team on a higher level architectually if needed.

I will usually just fix it and sign it off. Ideally you should either adopt a well-used style or write one so you can use tools such as clang-tidy.

Can I ask what type of company you work for? (Consulting, big IT corp., small-dev team part of non-IT company, etc.) Just curious since the way you describe how you work doesn't sound like your average in-house dedicated IT company.

Definitely not, it's a game studio that grew explosively afaik in the last year from just two people outsourcing to about 20 dedicated staff. It makes a lot of sense considering the circumstances.

Oh wow ok, then the way you describe the company makes a lot more sense, I think game devs tend to be much more lax with software engineering methodologies. I mean they shouldn't but still.

They're doing it wrong but its their headache not mine so its cool.