ariatemplates / hashspace

JavaScript client-side template engine
http://hashspace.ariatemplates.com
Apache License 2.0
14 stars 18 forks source link

Coding guidelines #57

Open olaf-k opened 10 years ago

olaf-k commented 10 years ago

This issue has been created after a post from @dpreussner on the at.com forum.

Since the code base is still at an early stage, we should take the opportunity to lay out a set of basic rules to follow so that code style stay consistent everywhere.

These rules could be listed in the (to be created) Contributing.md file.

PK1A commented 10 years ago

So, after working a very tiny bit on the #space code-base I also start to feel a need to formalize coding styles. The practical drive here is to minimize size of diffs from pull requests. If everyone uses different editor settings the code is going to be constantly re-formatted back and forth by different people.

Now, IMO, all those standardization efforts can succeed only if those are enforced automatically as part of the build. Non-conforming code should simply break a build.

Keeping the above in mind I would propose to introduce https://github.com/mdevils/node-jscs checks which is like jshint pushed a bit further. If we consider going down this path we should do it rather sooner than later as a bigger code-base will make it harder / impossible. BTW, those checks can be introduced on the folder-per-folder basis so there is a way of progressively introducing things.

WDYT?

divdavem commented 10 years ago

For code formatting, we could use grunt-jsbeautifier to format js code, as it is done in noder-js, see here. It has 2 modes: one to format files (to be used before committing changes to GitHub), and one to check that files are correctly formatted (to be used in travis-ci) so that we don't forget to format files.

marclaval commented 10 years ago

Definitively needed!

dpreussner commented 10 years ago

We´ll as I brought up the issue I +1 these efforts. But I would also suggest that a general style guide is used/defined which everyone adheres to while writing the code. This minimizes the effort to "fix"/normalize it afterwards. Most of it is just a matter of personal discipline, which of course can and should be aided by tools.

But in general it would be better not to have the different styles in the beginning. To cite your aria templates contribution guidelines:

Good code is readable maintainable reusable testable performant documented

jakub-g commented 10 years ago

Talking about the code style: If the rules are imposed by the automatic tools (jshint, style checkers/formatters), you'll learn the rules pretty quickly.

In my experience, it's waaay better to have a build-time rejections than any written guidelines because people don't read and/or forget the latter.

One little remark regarding grunt-jsbeautifier IMO it should be forked and changed to add empty newlines at the end of the files (which is a Unix convention) in order to be better compatible with Unix tools like vim which append the newline automatically, and for git not to display the ugly "no newline at the end of file" in diffs

dpreussner commented 10 years ago

How about a pull request first - before forking?

ymeine commented 10 years ago

Personally I prefer having a formal reference for coding style. Something easier to understand than a configuration file of a (often not exhaustive) formatting tool.

PK1A commented 10 years ago

@ymeine IMHO is something is not enforced automatically it will be neglected and not followed. Having formal / written guidelines are good (and I'm sure we can generate human readable description from a config file, if needed!) but if violations don't break the build we are going to catch some of those violations during code review and some others will be left unnoticed.

I really would like to automate all the things we can - I simply don't want to think about those rules while coding / doing code reviews.

jakub-g commented 10 years ago

On attester, we have a grunt task (not automatic, manual so far) that formats the code and launches jshint [1]. To get some insight about how this works, you can

  1. Do some development, check that your tests passes etc. then git commit;
  2. Launch this task from command line (grunt dev)
  3. git diff to see what the formatter has changed in your code (usually it will be things like adding/removing space in certain places, like after semicolons in object literals etc.)

It's as simple as that and you don't need a verbose document ;) Do this a few times and you'll learn the rules without reading anything.

IMO what we should rather discuss is what to choose :)

[1] https://github.com/attester/attester/blob/master/Gruntfile.js#L55 [2] https://github.com/ariatemplates/ariatemplates/issues/991#issuecomment-36112551 [3] https://github.com/millermedeiros/esformatter

ymeine commented 10 years ago

@PK1A Sorry, I forgot to precise that I prefer having that too! :smile:

Of course I'm in favor of automation, and +1 for generating something from a config file! However I think that a classical documentation format for coding style (in addition to the rest) allows talking about traits that go beyond what is covered by the tools we could use. Coding is not just a matter of syntax and indentation.

Well, personally I follow guidelines if I know there are some and where to find them... but I can't say it would be the case of everyone :stuck_out_tongue:

PK1A commented 10 years ago

@ymeine cool, looks like we are on the same page then :+1:

dpreussner commented 10 years ago

So what is the status on this matter now?

olaf-k commented 10 years ago

From what I gather we'll:

  1. come up with an automatic code "linter" (to run as a pre-commit hook)
  2. document its rules in a human-readable file. I think @PK1A is currently checking out the first point?
benouat commented 10 years ago

In terms of written coding guidelines, I found these ones to be really good to me https://github.com/airbnb/javascript

Have a look at it, it is huge, but every single usecase is illustrated with a example of bad/good snippet

PK1A commented 10 years ago

@dpreussner yes, I'm starting experimenting with https://github.com/mdevils/node-jscs

PK1A commented 10 years ago

Here is more info on jscs: https://yannick.cr/posts/enforcing-coding-rules-in-your-team-with-jscs/post

jakub-g commented 10 years ago

BTW apart from JSCS we can also add an .editorconfig file.

http://editorconfig.org/ is a project that aims to provide a simple configuration file which will be then read by a plugin for you IDE, which in turn transparently enforces the tabs vs spaces, trailing whitespace and all those kinds of conventions. It seems there are plugins for everything except Eclipse :)

dpreussner commented 10 years ago

@jakub-g Sounds like a neat idea.

What is the general status regarding the coding conventions? I think the project is past the "adjust manually" phase...