Cesium Coding Guide for Contributors

[pjcozzi]

Please provide technical feedback by end of Thursday:

• Coding Guide

Commit corrections, improvements, and new sections directly to this branch, coding-guide. Maintain the concise writing style and still try to provide context. If you don't have commit access, just leave a comment in this PR.

I went through much of the Cesium codebase and documented established practices in this guide. The scope of this PR is to finish and edit the guide, not to suggest changes to the current practices. Please open separate issues for those discussions or add them to #2524 (Cesium 2.0).

TODO

• Add TOC
• Remove the old JavaScript Coding Conventions on the wiki

[shunter]

Missing close } here

[pjcozzi]

Thanks, fixed. Feel free to just fix things like this. :)

[shunter]

I would suggest a different example here. Normally we would declare a file-scoped var for documentation reasons, and then return freezeObject(Blah); at the end. (this example is from a private enum)

[shunter]

One typical situation where private instance functions are useful is with our events, which accept function/scope pairs, in order to avoid allocating a closure just to bind this. Perhaps that's too esoteric to bother mentioning here.

[pjcozzi]

Up to you. Add it if you want. I have only used our events in one or two places.

[pjcozzi]

Reminder: please provide tech edits and feedback by end of today.

[mramato]

I've actually been tempted to open a pull request to fix this, there's never be a good reason for use strict to be in double quotes and it's one of the oddest things about our code base. I know you are just reporting what's already in the code here, just bringing this up in case anyone has an objection.

Also, you might want to mention that we use double quotes in HTML.

[mramato]

We should also mention somewhere near the top of the document that all Cesium code uses strict mode.

[pjcozzi]

I've actually been tempted to open a pull request to fix this

Good beginner issue: #3345.

[mramato]

I don't think it's immediately clear to readers why this is bad. Maybe explain in more detail that var c = cartesian gets removed at build time and changes the behavior of the code.

[mramato]

Good write up. Definitely better than our old one. I can add the Workers and Promises sections when I have spare time, but that's not something I can do right away. We should write up a GitHub issue for these things so we don't forget about them.

[pjcozzi]

• Avoid redundant nested property access. This

scene._state.isSkyAtmosphereVisible = true
scene._state.isSunVisible = true;
scene._state.isMoonVisible = false;

is better written as

var state = scene._state;
state.isSkyAtmosphereVisible = true
state.isSunVisible = true;
state.isMoonVisible = false;

[pjcozzi]

• Change example constructor functions in the guide to be, for example,

function Model(options) {

[pjcozzi]

Thanks for the feedback. Updated.

@slchow can you please copyedit this and open a pull request into this branch, coding-guide?

[pjcozzi]

This is ready to merge.