Coding standards within a small team


(Will Parker) #1

What does everyone use to manage coding standards across their projects / within a small team?

In my last role we had a git hook that ran our code through PHPCS and tidied it all up so we were all submitting code in the same style which was great and made commits really easy to read.

In my current role however we currently have no coding standards other than a loose PSR-2 requirement and nothing that validates or tidies up our code and all of our IDE’s (we all use different ones) seem to tidy up code differently.


(Roger Hutchings) #2

I briefly worked in a small team recently that had a set code style, and the linter to go with it, but no git hook. I wish we’d had that, or something like Travis set up for linting, as otherwise it just gets ignored…

Edit: pretty sure the backend guys where I work use Travis for linting as well as tests, but frontend don’t


(Matt Machell) #3

We use PHPStorm/IntelliJ/Webstorm, which has tools to automagically enforce your coding standards. It’s front-end support is pretty good, with 3 or so different linters.

One issue we’ve seen is that, auto cleanup causes the issue in that the code formatting changes get your name attached in version control, even if you didn’t do the original code, because the IDE “fixed” it before you committed.

-Matt


(Daniel Hollands) #4

This is only part of the greater question that you’re asking, but one thing which can help towards maintaining cohesive coding standard is a .editorconfig file which you use for all your projects.

We’re also using Code Climate, which in turn runs a number of other linter engines such as RuboCop (for ruby), as well as ones for other languages.

As for your specific question, although we use the above tools we don’t have a formal procedure for ensuring that they’re used to our best advantage. But as part of my job is to come up with the answers to the questions you’re asking, I’ll be keeping an eye on this thread with keen interest.


(Steve Jalim) #5

I apply a sensible amount of the Google Style Guide at my current client (yunojuno.com), via a link from our own developer docs.

This, however, is the only bit we explicitly quote, as it’s golden:

BE CONSISTENT.

If you are editing code, take a few minutes to look at the code
around you and determine its style. If they use spaces around all
their arithmetic operators, you should too. If their comments have
little boxes of hash marks around them, make your comments have
little boxes of hash marks around them too.

The point of having style guidelines is to have a common vocabulary
of coding so people can concentrate on what you are saying rather
than on how you are saying it. We present global style rules here so
people know the vocabulary, but local style is also important. If
code you add to a file looks drastically different from the existing
code around it, it throws readers out of their rhythm when they go
to read it. Avoid this.


(Will Parker) #6

Thanks for everyone’s feedback on this. Seems like there are a lot of different approaches out there and I’m going to have to decide what approach is the best fit for within my team.

I’ll do some more research and let you know when I’ve made a choice.


(Andy Wootton) #7

You could do the research but I wouldn’t try to force your opinion on the team without giving them an opportunity to change your mind. A standard agreed by the whole team is far more likely to be enthusiastically embraced.

Having said that, when I was at Nuclear Electric, there was an argument about indenting, on their discussion board, that had been going on for about 6 years :slight_smile:

I’d try to allow people to choose the tools they want too. Most teams will choose to follow a couple of people who make good choices. It’s competition/resilience vs efficiency/compatibility.


(Ben Paddock) #8

At Talis we have guidelines to help enforce coding standards at the code review stage: http://talis.github.io/topics/code-standards.html

We’re working on ways to automate as much of this as possible, where applicable (e.g. code linters).


(Marc Cooper) #9

I’ve been trying to stay out of this conversation. Mainly because folk have strong views about such things (and I have strong views too, which I might have expressed here once or twice :wink: ), and because I’ve worked in many, many places and have seen the best intentions wreak havoc in codebases.

My experience (and I’m old) is that many of our pithy statements of “good practice” can be misunderstood (usually through lack of experience and exposure to truly maintainable code – I’m avoiding the use of “high quality” code, because, like driving skills, everyone believes they are in the best 50% :slight_smile: ).

Perhaps the most commonly misunderstood pithy principle is DRY, mainly due to lack of understanding/awareness/comprehension of the difference between syntactic and semantic duplication. It’s a far more subtle principle that it first appears.

Lack of understanding of DRY, but applying it verbatim, has created many a ball of string I’ve helped untangle; often to the bemusement of some developers.

With OO languages, I’d wager the most important principle is “Tell, don’t ask”. For sure, all the SOLID principles are good, but if you adhere to “Tell, don’t ask” (i.e. use objects as objects), you’ll probably have a codebase that can be refactored without too much pain.

The least important, and the most disruptive practice, ime, is enforced linting. This is imperialstic OCD imposition :wink: If you fly into a foaming-mouthed frenzy at someone writing if ( ! blah) instead of if (! blah), then you should probably change career. I’ve seen more hours spent over discussions of such heinous whitespace crimes than the provacateurs would ever spend learning to understand DRY or SOLID or “Tell, don’t ask”.

btw, if you run PSR-2 linters, then it will “fail” your code for if ( ! blah) instead of if (! blah). What a fabulous waste of someone’s time that is.

The principles @pads linked to seem pretty sensible to me. The only problem being that a lot of devs won’t really understand them. I know they think they will – I work with devs too – but they mostly don’t. Folk mean well, but principles like those I’ve mentioned were formulated by folk who’ve spent their lives messing with software. You can’t distil that experience into pithy sentences nor a wikipedia page.

Linting, ime, is the work of the devil. Devs care. Devs mean well. Most devs are really keen to learn, if folk take the time to coach. They’ll create a consistent codebase without imperialstic OCD imposition, and be happier for it.

I also have strong opinions on this subject which I’d be happy to share :slight_smile:


(Marc Cooper) #10

My advice: don’t. See something in the code that fills you with rage? Change it. Commit. Push. Feel the glow of satisfaction of "doing it right"™

If your devs can’t conform to a consistent code base, then coach them.

Spellchecks don’t teach you to spell. Linters don’t help you write maintainable code.


(Steve Jalim) #11

Deliberately dodging discussing most of @auxbuss’s post because I’m brain-dead today - I will chip in this, though:

Agreed, they don’t make code more maintainable from a refactoring POV, but they DO (in my experience) reduce defect rates. Lint-free code (when used with appropriately configured linters) mean facepalm-style things like unbound or unused vars get caught immediately - even before tests are run in most cases. The manual fixing of merge conflicts also has one dimension of hurt removed from it.


(Richard Cunningham) #12

I think it’s important to ask the team and get ownership of the problem from them.

In addition to the above comments, In PHP projects I always turn on all errors:
error_reporting( E_ALL );
Have this report to web browser when in development and via email when in production, which can pick up on many types of coding mistakes.

In javascript I use “use strict” to pick up some problems, such as not initializing variables.


(Marc Cooper) #13

Those things are good, @stevejalim, although I was speaking of coding standards rather than static analysis in general, of which I am a big advocate. I spent a lot of Friday looking at code-maat output for a project.


(Ben Paddock) #14

I just think that machines are pretty good at nagging about things to do with code that do not require context and are unambiguous. I realise that this does not cover much ground but it’s better than nothing. I would also agree that linting can be disruptive - the further down the road the project is the more pain it is with many many lines of code to change. However once it is in I think having that kind of foundation helps in the long run.

For me personally, my coding standards will get better with experience and forming good habits. I am comfortable with receiving strong (but fair) criticism from both humans and machines because I know no better at the time.