Skip to content
This repository

Code Style Guide #2

Closed
lukeab opened this Issue · 19 comments

7 participants

Luke Ashe-Browne Shane Quigley dominickm Marcus Karlsson Sean McArdle Nehemiah I. Dacres Daniel Samson
Luke Ashe-Browne
lukeab commented

in keeping with the guidance of the project team/volunteers, a code style guide for the various elements of the project would be useful.

HTML markup style
css philosophy
javascript guide

...other stuff :)

dominickm
Owner

Great idea. I am working on a basic style guide for the JS. Mostly just to keep consistency between all of the code. Some other thoughts are I think we should mandate that semicolons be used if possible; the idea would be to avoid 'mystery meat' issues.

Luke Ashe-Browne
lukeab commented

That for a start.

Often i refer back to the Crockford wisdom for my practices. Single var statements per scope, and do some modularising of code to avoid messing up public scope, and reduce the search overhead when looking up the chain scopes.

Scope scopedy scope scope.

One other idea is to pass code through jslint in some kind of moderate mode to gain some benefit of it's guidance.

Many more things to add to this list, but it's a start

dominickm
Owner

@lukeab I just put some stuff in the style guid go ahead and take a look. If you have edits / additions fire of a pull request. Hopefully we can get most of this house keeping stuff hammered out in the next day or so.

Marcus Karlsson

Good idea to take this initiative early on. The topic comes up anyway sooner or later.

Personally I would prefer that lines are at most 72 characters long. Especially documentation tends to otherwise have very long lines which doesn't work very well with most diff tools. Longer lines will usually either be cut off or make it necessary to scroll back and forth which makes larger diffs relatively difficult to read. This will also make things like git-log work well in an 80 character terminal.

The question of tabs or spaces will probably come up too. I don't really have an opinion there. Using tabs and interpret them as up to eight spaces tends to be somewhat universal though.

Luke Ashe-Browne
lukeab commented

I think my pythonic history makes me revile tabs, damn evil things. I would almost always work in 2 or 4 spaces instead of tabs, and most editors have 4 as the default also.

dominickm
Owner

Yeah, I don't have a strong opinion on the tabs V spaces thing, but I think you are write about a line legth limit @marcusk.

Maybe we should just pick something on tbs v spaces.

Luke Ashe-Browne
lukeab commented

1 tab => 4 spaces, as far as i have seen that's the widest adoption.

dominickm
Owner

@lukeab Yeah, that is the one I see most commonly as well. Unless there is some legitimate reason to reconsider, I'll add that as a requirement in the current doc.

Sean McArdle
sean-m commented

On the semicolon issue, jslint checks for proper semicolon use and other things that mainly relate to code style. Do you think requiring that commits pass jslint or maybe jshint before a merge is a good idea?

dominickm
Owner

@sean-m I am actually not familiar with JSlint let me look into it a bit. Overall, I think it is a good idea to not require specific tools, since everyone has their own toolchain they like.

Anyone else ever use JSlint?

Luke Ashe-Browne
lukeab commented

I would try to stick with JSLint, some people believe it's too strict, and Hint exists as a more lenient option, but i think Lint's strictness is nice.

That said Hint is trying to be lenient on things the creators deem not important in terms of good habbit or performance, then again, the multiple var statements per scope being allowed in Hint is an issue I would argue against hint due to.

So we can proceed with either, and see if they help contributors maintain their code standard or just become too hard for less experienced members of our community to use.

Perhaps this is something that the project luitenants build into a feedback loop when accepting pull requests.

dominickm
Owner

@lukeab et al Yeah, I took some time running some of my JS through JSLint looks pretty good and would certainly help maintain a standard. Let's go with that unless there is a big downside to it.

Nehemiah I. Dacres

there are tools to turn tabs into spaces, vi has a setting

Daniel Samson

I can't get JS Lint to work with jQuery. I pass with regular JavaScript code.

dominickm
Owner

@Techfix Hmm... I will have to look into that. JSLint is new to me also, so that may just be a limitation of the tool.

Luke Ashe-Browne
lukeab commented

if you pass just a bare js file through with $ functions and no decleration of $, it usually freaks out. You can solve this with a closure amd immediate execution

(function($){
  $(document).ready(function(){
    //stuff in here
  });
}(jQuery));

If you keep your code well formed it's usually fine.

dominickm
Owner

@lukeab Good to know thanks for the knowledge drop. @Techfix Does that make sense to you?

Daniel Samson

Well I tried in JS Lint and I still get errors. It does not support jQuery. I am using http://www.jslint.com/ are you using something else?

Shane Quigley
Collaborator

Hey did a pull and its nice to see the rss feed but the code wasn't jslint compliant and the style used for the objects in the jquery.jb.js is different

    url : {
        "get" : function () {
            "use strict";
        }
    },

look at the different use of quotes and lack there of

    feed : {
        get : function () {
            "use strict";
            var callback = function () {
                "use strict";
                $('div#feed-holder > ul#feed-list').listview();

I'll change it to be jslint compliant when I commit up my stuff but please be more careful. If you have question about jslint or the style guide just ask them. I think everyone understands that this is a learning experience and no one is going to make fun of you for not knowing something. Also using url in the feed object like that is a little dodgy considering there is a url in the jb object.

Shane Quigley ShaneQful closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.