Styleguides

Since there’s no discussion around it, I will start the vote for the Ruby styleguide tomorrow, unless discussion emerges until then.

Proposal: Ruby styleguide

Adopt bbatsovs styleguide with the derivations enforced by this Rubocop config and the additional derivations mentioned in the description of this discussion.


Outcome: The proposal passed.

Votes:

  • Yes: 4
  • Abstain: 3
  • No: 0
  • Block: 0

Note: This proposal was imported from Loomio. Vote details, some comments and metadata were not imported. Click here to view the proposal with all details on Loomio.

@dumitruursu I recently found Kernel#abort, which is the official way to abort the program with an error state.

hmm, that is actually not bad.

In the proposed settings for jshint I changed
camelcase : true: we already use camelcase for most backbone.js functions
quotmark: 'single': no need to escape double quotes in HTML
maxlen: 120

I will create a proposal for the javascript styleguide as soon as the ruby proposal closes.

So there’s no possibility to differentiate camelCase for class names and function and snake_case for local variables ? Well. I’ll deal with it.

Proposal: Javascript Styleguide

Enforce the javascript styleguide as described in the discussion and implemented by the following JSHint config: https://gist.github.com/svbergerem/83f5d267cc1897d64edd


Outcome: The proposal passed.

Votes:

  • Yes: 7
  • Abstain: 2
  • No: 0
  • Block: 0

Note: This proposal was imported from Loomio. Vote details, some comments and metadata were not imported. Click here to view the proposal with all details on Loomio.

In addition, I’d propose to use JS Code Style to enforce some rules like no trailing spaces.

In addition, I’d propose to use JS Code Style to enforce some rules like no trailing spaces.

It is absolutely unnecessary to use two tools doing basically the same. JSHint does everything we need. Trailing whitespace should be removed anyway, but that’s nothing JS specific. :wink:

In addition, I’d propose to use JS Code Style to enforce some rules like no trailing spaces.

I agree. The JSHint docs say that some options will be removed in the next major release and that you should use JSCS for those instead. Unfortunately Hound doesn’t support JSCS yet and as far as I know there is no Ruby gem for JSCS.

@dennisschubert : as @steffenvanbergerem said, the options related to code style are going to be deprecated in the next release of JSHint. They say that JSHint is only about code correctness and not code style. JS has a lot of ways to do some things and I saw very different ways of developing while working on the notifications system. We need to agree on things like using var that = this; or rather var self = this;, spaces usage (before and/or after brackets and curly braces?) and so on. Every big project has a code style. It improve the understandability of the code as it provides conventions.

@dennisschubert @jasonrobinson If you want we can have another proposal after the current one to decide whether we prefer single or double quotes. In that case I would keep the current value (false, accept single and double quotes) until we have an outcome for the second proposal.

It would be a bad idea to enforce single quotes if the majority prefers double quotes.

Proposal: Javascript Styleguide: Use double quotes

Change the approved javascript styleguide by using double quotes instead of single quotes.


Outcome: We will use double quotes.

Votes:

  • Yes: 3
  • Abstain: 0
  • No: 1
  • Block: 0

Note: This proposal was imported from Loomio. Vote details, some comments and metadata were not imported. Click here to view the proposal with all details on Loomio.

For (S)CSS I suggest adopting the coding styles from Bootstrap, which are very reasonable, and will provide enough structure to make the code consistent.

Also, it will be very useful to have a similar notation on the bootstrap styles and our own, with no modular__classNaming–nonsense.

http://codeguide.co/#css

I think it will be pretty easy to adopt too.

This is only for the “measurable” styleguide, there are also some best practices we should adopt, but those will be our own, and we can add best practices gradually on the run as we find use cases. I have a few of my own to suggest.

@steffenvanbergerem if you guys want me to create a separate proposal, just let me know. I see this one is closed, but I guess the discussion is still alive.

A discussion can totally have multiple proposals, we already used this one to decide Ruby and Javascript. Feel free to edit the top post accordingly too to reflect current discussion status.

I’d like to enforce the styleguide with HoundCI, at least as much of it as possible. It runs SCSS-Lint, do you happen to run across an existing config for the guide you propose?

Nope, but I’ll give it a try.

There are some further insights on the approach that gave birth to that styleguide here, the guys at Github use SCSS and the linter too:

http://markdotto.com/2014/07/23/githubs-css/#linting

I still have to check some config values to see if they are fully compatible with the guide I posted, but for anyone interested in taking a look, here is the ANSI-colored output of SCSS Lint with Hound’s config, which I think is pretty close to the guide:

http://www.vispress.com/scss-lint-hound.txt

And here are the differences between the default scss-lint config and Hound’s:

https://www.diffchecker.com/n2wj07ys

I think maybe we should disable some rules, or apply some automatic styling, even using sed…

As I pointed it here one of the things that gave me hard time whil porting to BS3 was the fact that classes where used both to define CSS rules and to select in JS.

As most of the CSS rules defined could be removed because they were alreay defined by BS, I nerver knew if the class ws safe to remove or if it was still used by any JS somewhere.

So, my idea would be, from now, to avoid using the same classes to define CSS rules and to select in JS. The diference could be pointed by the use of a prefix on the classes used by JS like js-. This will avoid to break logic or unit tests in the future when removing deprecated classes.

For SCSS I think we could start with the default config of scss-lint. Any comments before I create a new proposal?

Proposal: SCSS Styleguide

Use scss-lint with the default config: https://github.com/brigade/scss-lint/blob/master/config/default.yml

Detailed information about the different linters: https://github.com/brigade/scss-lint/blob/master/lib/scss_lint/linter/README.md


Outcome: We will use scss-lint with the default config.

Votes:

  • Yes: 6
  • Abstain: 0
  • No: 0
  • Block: 0

Note: This proposal was imported from Loomio. Vote details, some comments and metadata were not imported. Click here to view the proposal with all details on Loomio.