dominickm / jupiter_broadcasting_mobile_community

The Jupiter Broadcasting community project.
Other
100 stars 17 forks source link

Code Style Guide #2

Closed lukeab closed 11 years ago

lukeab commented 12 years ago

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 commented 12 years ago

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.

lukeab commented 12 years ago

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 commented 12 years ago

@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.

ghost commented 12 years ago

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.

lukeab commented 12 years ago

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 commented 12 years ago

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.

lukeab commented 12 years ago

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

dominickm commented 12 years ago

@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-m commented 11 years ago

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 commented 11 years ago

@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?

lukeab commented 11 years ago

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 commented 11 years ago

@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.

dacresni commented 11 years ago

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

ghost commented 11 years ago

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

dominickm commented 11 years ago

@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.

lukeab commented 11 years ago

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 commented 11 years ago

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

ghost commented 11 years ago

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?

ShaneQful commented 11 years ago

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.