Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Minor updates for the Coding Guide #12452

Open
javagl opened this issue Jan 26, 2025 · 0 comments
Open

Minor updates for the Coding Guide #12452

javagl opened this issue Jan 26, 2025 · 0 comments

Comments

@javagl
Copy link
Contributor

javagl commented Jan 26, 2025

The current CodingGuide contains a few recommendations that may have to be updated.

One specific point that was discussed as part of a recent PR is whether the Coding Guide should contain a recommendation like

Don't use Array.push.apply() for large arrays. Use addAllToArray or the [...spread] syntax instead
(Maybe with details about where the spread syntax should also be avoided...)

I looked at the the Coding Guide, thinking about where something like this might be added. But there are two and a half issues with that:

  1. There are some plainly outdated statements in the Coding Guide
  2. Some recommendations there are obsolete, because they already are enforced with linting rules
    2½. The specific case of push.apply is already covered with a newly introduced linting rule

I think that very roughly speaking, the Coding Guide should not repeat things that are covered by the linter. (One could argue that it could explain why these rules exist, but that's a slippery slope, and might bloat it up to a point where nobody is reading it any more...)

Some preliminary points that I noticed and that might have to be updated accordingly, starting at the the Basic Code Construction section:

  • "Cesium uses JavaScript's strict mode, so each module (file) contains "use strict"
    • That's as wrong as it can be. No file contains this anymore 🙂
  • "To avoid type coercion..."
  • "let and const variables have block-level scope"
  • "Modern language features may provide handy shortcuts and cleaner syntax, ..."
    • This is a bit vague. But the push.apply-to-[...spread] (with caveats) might fit here...
  • "Do not use an unnecessary else block at the end of a function..."

Some further (potential) updates are connected to open issues or pending PRs - just a quick summary:

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant