From 713ba25fe3813b4b7a93e0e2815b71c6db0880b0 Mon Sep 17 00:00:00 2001 From: Mike Ball Date: Fri, 18 Feb 2011 09:39:52 -0500 Subject: [PATCH] updates to committer and style guides... --- source/commit_code.textile | 84 +++++++++++++++++++++++++++++++++----- source/style_guide.textile | 12 ++++-- 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/source/commit_code.textile b/source/commit_code.textile index a90ba82..8200100 100644 --- a/source/commit_code.textile +++ b/source/commit_code.textile @@ -1,16 +1,24 @@ -h2. Commit Code +h2. Committing Code -So, you're ready to push some code into the public repo? Here's a short list of things you can do to make sure your code gets past the review process. +So, you're ready to push some code into the public repo? Here's a list of things you can do to make sure your code gets past the review process. endprologue. h3. Conform to SC Style Guides -Yeah, this is pretty obvious. +Yeah, this is pretty obvious. If you haven't already go "read them":style_guide.html. -h4. Must Pass JS lint +h3. Must Pass JS lint -JS Lint checks for easy mistakes: undefined global vars, extra or missing commas, etc... All code must pass JS Lint with the following config: +Javascript can be beautiful and annoying all at the same time. "Crockford's JSLint":http://www.jslint.com is a god send and helps check for common mistakes like: + +* Undefined global vars +* use of +==+ and +!=+ instead of +===+ and +!==+ +* Extra or missing commas + +The following JS lint config can be used in combination with the "SproutCore Textmate bundle":https://github.com/sproutit/sproutcore-tmbundle + +Install it at +/SproutCore.tmbundle/Support/bin/prefs.js+ var JSLINT_PREFS = { adsafe : false, // if ADsafe should be enforced @@ -41,18 +49,74 @@ var JSLINT_PREFS = { -h4. All unit tests must pass +TODO: we should have a command line tool that is included in the build tools so there is a consistent standard and so all the cool kids using VI have an option + +h3. Unit Tests + +h4. Don't break anyone else's tests + +On stable builds all the unit tests should pass. On development branches if there are broken tests because of a major project that someone on the Core Team is doing thats you can ignore those failures. + +h4. Write good tests + +Writing good unit tests is something often pontificated on by seasoned developers so rather than waste time with a long essay I'll just show some examples of good and bad unit tests: + + +h5. View Unit Tests + + +test("basic", function() { + var view = pane.view('basic'); + ok(!view.$().hasClass('disabled'), 'should not have disabled class'); + ok(!view.$().hasClass('sel'), 'should not have sel class'); + + var input = view.$(); + equals(input.attr('aria-checked'), 'false', 'input should not be checked'); + + var label = view.$('span.label'); + equals(label.text(), 'Hello World', 'should have label'); +}); + +This test is great because it verifies that the *rendered DOM* is correct + + +test("basic", function() { + var view = pane.view('basic'); + equals(view.get('title'), 'Hello World', 'should have label'); +}); + +This test sucks because it only verifies that you can set a property on an object. + + +TODO: get some more examples... + + + +h4. Running unit tests + +You can run the unit tests by using the test runner app +http://localhost:4020/sproutcore/tests+ or by calling them manually: + ++http://localhost:4020/"framework or app name"/en/current/tests.html+ -> all the tests ++http://localhost:4020/"framework or app name"/en/current/tests/path/to/your/tests.html+ -> run a specific set + +For example: +All of SproutCore's datastore tests: ++http://localhost:4020/sproutcore/datastore/en/current/tests.html+ + +Just the tests in the store directory ++http://localhost:4020/sproutcore/datastore/en/current/tests/system/store.html+ - • Must add tests when adding classes and methods +Just the find tests ++http://localhost:4020/sproutcore/datastore/en/current/tests/system/store/find.html+ -h4. Consistency +h3. Consistency All code must be consistent with patterns found in the Application and within the Frameworks. example: MVC, Visitor, Statecharts Don't add special functionality just for shits and giggles! -h4. Performance +h3. Performance Avoid Known Slow stuff: -.observes @@ -62,7 +126,7 @@ Avoid Known Slow stuff: Code must be prove-ably fast with SC.Benchmark Keep in mind code size in kb, number of statements(IE!), doing too much on init -h4. Maintainability +h3. Maintainability • Don't override private APIs • Be consistent and predictable diff --git a/source/style_guide.textile b/source/style_guide.textile index 7c6e53d..7e3fa57 100644 --- a/source/style_guide.textile +++ b/source/style_guide.textile @@ -1,10 +1,10 @@ h2. JavaScript Style Guide -This guide comes from http://wiki.sproutcore.com/w/page/12412942/JavaScript-Style-Guide +Coding and naming convention style guidelines make your code clearer, easier to read and, they can also actually improve performance. -Coding and naming convention style guidelines not only make your code clearer and easier to read, but they can also actually improve performance. Although you can use any convention that makes sense for you, the guidelines described here are based on years of experience from SproutCore developers in writing understandable and maintainable code. +Although you can use any convention that makes sense for you, the guidelines described here are based on years of experience from SproutCore developers in writing understandable and maintainable code. -The SproutCore coding and naming convention style guidelines are based on the Cocoa style guide from "CocoaDevCentral":http://cocoadevcentral.com/. Being a framework that is based on JavaScript, not everything in the aforementioned style guide applies completely. However, since SproutCore has a lot of conceptual similarities with Objective-C, it is useful to align the coding styles somewhat. It not only allows for the similar concepts to be described in the same way, but also it makes it easier for developers using both Objective-C and SproutCore to go from one to the other. Plus, the Cocoa coding style guidelines are widely followed and therefore there is a pool of developers already familiar with it. +The SproutCore coding and naming convention style guidelines are based on the Cocoa style guide from "CocoaDevCentral":http://cocoadevcentral.com/. endprologue. @@ -313,4 +313,8 @@ h3. Related Material "Google Tech Talk regarding Javascript performance":http://www.youtube.com/watch?v=mHtdZgou0qU -"Javascript: The Good Parts":http://www.youtube.com/watch?v=hQVTIJBZook \ No newline at end of file +"Javascript: The Good Parts":http://www.youtube.com/watch?v=hQVTIJBZook + +h3. Credit + +This guide came mainly from the old "wiki":http://wiki.sproutcore.com/w/page/12412942/JavaScript-Style-Guide and was written by Peter Bergström