Skip to content
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

Add optional parameters notation to JSDoc #15205

Merged
merged 1 commit into from
Jun 15, 2024
Merged

Conversation

arista-ms
Copy link
Contributor

Closure is failing with a message like:

ERROR in abc.js:48234 (originally at @babylonjs-core@7.10.0-e5902e94f7a1ba005e89/dev/core/src/Misc/observable.ts:99) from closure-compiler: Inline JSDoc on default parameters must be marked as optional * Defines the current scope used to restore the JS context ^^^^^^^

Adding these comments to the JSDoc fixes that

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 14, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@deltakosh deltakosh enabled auto-merge (squash) June 15, 2024 00:01
@bjsplat
Copy link
Collaborator

bjsplat commented Jun 15, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 15, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Jun 15, 2024

@deltakosh deltakosh merged commit 9094099 into BabylonJS:master Jun 15, 2024
11 of 12 checks passed
@sebavan
Copy link
Member

sebavan commented Jun 17, 2024

@RaananW, it looks like we might have some weird specific requirements due to the use of closure compiler... I wonder whether there are some ways to automate it or @arista-ms if we should skip testing those typings in the build. Being based on JSDoc only is kind of scary considering the typings are ok ?

@RaananW
Copy link
Member

RaananW commented Jun 17, 2024

we are tsdoc conform, checking against jsdoc would require a lot of changes on our side. I would rather understand what the requirements are before merging a lot of those changes. We might be able to auto-fix them, or make sure we are conform some other way. I just don't know the (build) system's requirements and properties :-)

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

Successfully merging this pull request may close these issues.

5 participants