Styleguides

[faldrian]

Of course! https://twitter.com/mathias/status/247708186696634368

(No, there is not even a performance difference … http://jsperf.com/single-quote-vs-double-quote/3 )

[augier]

Any such reason for preferring ' in JS?

None, it is just personnal preference. And I prefer using " " for class or id when I define inline HTML, so this prevent quotes collision and usage of \".

Edit : yeah, just like @faldrian

Also, about JS, I like JSON rather that function() for object definitions :

JSON :

var object = {
    var1: 'var',
    var2: 'var also'
}

Function definition :

var object = function(){
    var1 = 'var';
    var2 = 'var also;
}

Prettier and easier to read.

[augier]

Would be nice to set option multistr to true, Milou bothers me with that >.<

[svbergerem (Steffen van Bergerem)]

The JSHint docs say

Multi-line strings can be dangerous in JavaScript because all hell breaks loose if you accidentally put a whitespace in between the escape character () and a new line.

[augier]

I didn’t know that. Can’t wait for ES6…

[jhass (Jonne Haß)]

So, shall we put mandatory comments to a vote?

[faldrian]

Since we exchanged arguments with pretty much no effect, this might be a good idea.

[augier]

I think so.

To me, proposition should at least include these elements :

• mandatory comments required for eavery PR
• at least for JS code
• at least for functions that are above 3 lines.

[jhass (Jonne Haß)]

mandatory comments required for eavery PR
at least for functions that are above 3 lines.

That’s contradictory, please clarify.

at least for JS code

All or nothing, doing it for part of the code base and for another part not is just worse.

[augier]

I say at least, now if we vote fore mandatory comments and you think it’s better to

That’s contradictory, please clarify.

Every new function has to be documented unless it is obvious (less than 3 instructions) eg. this is obvious and this too.

I consider as an instruction any call of method. So, if there’s chained methods call, each call is an instruction. Eg :

var.call1().call2().call3().call4()

is 4 instructions. But I’d like to limit a too big number of chained call (not more than 4, maybe ?) if possible.

Also, about SCSS is nesting limit relevant ?

[jhass (Jonne Haß)]

That’s way too vague, you can’t measure complexity in LoC or calls.

[augier]

It’s just a proposition, I don’t see any other easy way to measure it. It’s not useful to put mandatory comments for very obvious case, but we need a clear rule to determine what is ovious and what is not.

[faldrian]

I don’t like rules and hard limits where none are necessary. It is already possible to look through new pull requests and add comments like “hm… big block of code, care to comment some of that?” - if the developer not already has done so.
We all want to write good and understandable code - if there are people who have hints how to make code better understandable, they can say so in the PRs.

I think the solution to documentation issues will more likely be solved by communication and external documentation than by enforcing arbitrary rules on the structure of new code.

Let’s focus more on gettings things done here than setting up a ruleset to play by… (that won’t be used if people don’t see the purpose of it).

[jhass (Jonne Haß)]

I’m totally with Faldrian. Since there obviously are no simple rules to determine when a mandatory comment should be placed, do you still hold to wanting them?

[augier]

We have to find how to better document the code. This is the solution I found. If I’m the only one on this, of course it will never be accepted.

But the problem is still here : the code is still abstruse. So, no matter what, we need a solution on this.

[augier]

When the styleguide is set, what if I set up a RubyMine inspection file for the official repo ?

[jhass (Jonne Haß)]

But the problem is still here : the code is still abstruse. So, no matter what, we need a solution on this.

The way forward is to make it less abstruse, not to bury the clear parts in comments.

When the styleguide is set, what if I set up a RubyMine inspection file for the official repo ?

I don’t use RubyMine, so maybe. We will most likely add the above mentioned linters with configs that match the styleguide as good as possible and enable Hound.

[augier]

I don’t use RubyMine, so maybe. We will most likely add the above mentioned linters with configs that match the styleguide as good as possible and enable Hound.

BTW, RM supports JSHinter and JSLinter.

[dumitruursu]

JSLint is a bunch of cr*p. It’s too harsh for any sane person. JSHint is the way to go, I guess. I think comments should be optional, but each file should at least have a comment, explaining what it’s trying to do in the system, why it’s needed there. But this can be enforced when accepting PR’s, there’s no need for a linter to check it.

Regarding SCSS - It’s a mess right now. I’ve been thinking a lot about it, and it’s a difficult problem.

I propose to use the SMACSS approach and use these rules to separate the selectors:
https://smacss.com/book/categorizing

We should have 5 folders and a file:

• base

  • _base.scss (only for including this folder files)
  • random .scss files

• layout

  • _layout.scss

  • the same rules as before

• module

• state

• themes

  • default (a folder)

    • _main.scss (to include the other files) (this should be nested under default folder, it’s something weird with loomio markdown)
    • whatever files the theme creator wanted, it may again follow the SMACSS structure

• application.scss

@import ‘base/base’, ‘layout/layout’, ‘module/module’, ‘state/state’;
@import ‘themes/cerulean/_main’; (changed the theme to cerulean)

[svbergerem (Steffen van Bergerem)]

@dumitruursu We already use JSHint.