Skip to content

Ongoing documentation fixes #11749

New issue

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

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

Already on GitHub? Sign in to your account

Open
5 tasks done
javagl opened this issue Jan 9, 2024 · 12 comments
Open
5 tasks done

Ongoing documentation fixes #11749

javagl opened this issue Jan 9, 2024 · 12 comments
Labels
category - doc good first issue An opportunity for first time contributors onramping

Comments

@javagl
Copy link
Contributor

javagl commented Jan 9, 2024

There occasionally are "small" fixes for the generated docmentation. These range from "fixing typos" to "fixing broken links" or "fixing type annotations". These usually do not involve rigorous tests, because this only refers to fixes that do not affect the functionality. A corner case is that of TSDoc fixes that may affect the generated typings. These may warrant some scrutiny, but still are not covered by real specs, but maybe only by a few lines in the Specs/TypeScript/index.ts file.

I'd suggest listing these issues here. Everybody can add comments with things that have to be fixed, and we can try to keep track of the things that have been addressed using GitHub [ ] Task checkboxes.

  • The Cesium3DTileset.from... functions claim to throw a DeveloperError when the tileset version is not 0.0 or 1.0. (E.g. fromUrl and fromIonAssetId).
    Suggested fix: This should include 1.1 as a valid version number.

  • These functions are actually not throwing a DeveloperError, but a RuntimeError, as of this check.
    Suggested fix: Considering that the developer does not have influence on whether ~"the data is valid", it should probably be a RuntimeError (and documented accordingly)

  • The ...GeometryUpdater#isDynamic property documentation contains invalid links. (For example, the PolylineGeometryUpdater documentation, generated from PolylineGeometryUpdater, but also other updater classes).
    Suggested fix: It should just omit the part that says

     * If true, all visualization is delegated to the {@link DynamicGeometryUpdater}
     * returned by GeometryUpdater#createDynamicUpdater.
    

    This is an implementation detail that is not relevant for the reader of the documentation.

  • The EntityCollection#add function claims to throw a DeveloperError for duplicate IDs, but actually throws a RuntimeError.
    Suggested fix: It should be a DeveloperError. The developer should make sure that there are no duplicate IDs. (In doubt, the developer can trivially check whether an ID is already present)

  • The orientation property in the entity constructor options is documented to be of type Property, but it can also be a Quaternion.
    Suggested fix: Change the type to {Property | Quaternion}. Investigate whether the same applies to other properties there.

@javagl
Copy link
Contributor Author

javagl commented Feb 17, 2024

  • There are many places that define a specific type for a certain property (like Cartesian3), but they also define a default value that may be undefined. This is an inconsistency insofar that it's not possible to assign undefined there, and the type of the property should have been Cartesian3 | undefined to begin with. One example (and a regex for finding others) is mentioned in the issue around Can't unset constrainedAxis in typescript without casting  #11475 (comment)
    Suggested fix: Add ... | undefined in all these places

  • It's not really a "documentation" fix, but ... way not warrant a full-fledged PR: I think that this updateStages is not used
    Suggested fix: Remove it...

@Tony3141Hui
Copy link

  • There are many places that define a specific type for a certain property (like Cartesian3), but they also define a default value that may be undefined. This is an inconsistency insofar that it's not possible to assign undefined there, and the type of the property should have been Cartesian3 | undefined to begin with. One example (and a regex for finding others) is mentioned in the issue around Can't unset constrainedAxis in typescript without casting  #11475 (comment)
    Suggested fix: Add ... | undefined in all these places

Analysing ./pakages/**

@default\s+(undefined|null)\s*\n([^/]*\n)*.*@type\s+

No results, means no @type after @default in same annotation block.

@type\s+.*\n([^/]*\n)*.*@default\s+(undefined|null)\s*\n

100+ results, means 100+ targets.

@type\s+.*(undefined|null).*\n([^/]*\n)*.*@default\s+(undefined|null)\s*\n

5 results, means 5 correct, including fixed #11475, in 100+ targets.

@javagl
Copy link
Contributor Author

javagl commented Sep 11, 2024

(State of 1.121)

@javagl
Copy link
Contributor Author

javagl commented Sep 28, 2024

Not really a "documentation" fix, but ...

@javagl
Copy link
Contributor Author

javagl commented Sep 30, 2024

Also not a "documentation" fix, but maybe part of a cleanup that does not affect functionality:

  • I'm pretty sure that frameState.light is not used. If it really is not used, it could be removed.

@javagl
Copy link
Contributor Author

javagl commented Nov 28, 2024

@javagl
Copy link
Contributor Author

javagl commented Dec 18, 2024

@javagl
Copy link
Contributor Author

javagl commented Feb 14, 2025

@javagl
Copy link
Contributor Author

javagl commented Feb 18, 2025

  • The Axis class and the associated ..._UP_TO_..._UP matrices do not appear in the documentation.

@javagl
Copy link
Contributor Author

javagl commented Mar 27, 2025

@javagl
Copy link
Contributor Author

javagl commented Apr 5, 2025

Not actually a documentation fix, but rather a bug fix:

@javagl
Copy link
Contributor Author

javagl commented Apr 13, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category - doc good first issue An opportunity for first time contributors onramping
Projects
None yet
Development

No branches or pull requests

3 participants