Naming conventions - linting #16455
Naming conventions - linting #16455
Conversation
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
Reviewer - this PR has made changes to one or more package.json files. |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16455/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16455/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
| @@ -10,8 +10,8 @@ import type { Observer } from "core/Misc/observable"; | |||
| import { Logger } from "core/Misc/logger"; | |||
| import type { AbstractEngine } from "core/Engines"; | |||
|
|
|||
| const _positionUpdateFailMessage = "Failed to update html mesh renderer position due to failure to get canvas rect. HtmlMesh instances may not render correctly"; | |||
| const babylonUnitsToPixels = 100; | |||
| const _PositionUpdateFailMessage = "Failed to update html mesh renderer position due to failure to get canvas rect. HtmlMesh instances may not render correctly"; | |||
There was a problem hiding this comment.
Does this need an underscore?
There was a problem hiding this comment.
No, this was an old rule that was changed and will be reverted tomorrow. Same with all of those that I didn't revert
Sorry - no, we don't need the underscore, but those had underscore already. No underscore was added, it is just the unchanged defined naming convention. I will remove all underscores from non-exported global members to stay consistent and will remove the underscore from the rule
packages/dev/addons/src/htmlMesh/pointerEventsCaptureBehavior.ts
Outdated
Show resolved
Hide resolved
| @@ -297,7 +297,7 @@ export class HtmlMeshRenderer { | |||
| } | |||
|
|
|||
| // prettier-ignore | |||
| protected _getCameraCSSMatrix(matrix: Matrix): string { | |||
| protected _getCameraCssMatrix(matrix: Matrix): string { | |||
There was a problem hiding this comment.
Our coding standards doc (not public) says to use all caps for initialisms. Can linting even tell the difference between initialisms and abbreviations? Maybe we need to modify the doc.
There was a problem hiding this comment.
Abbreviations are an interesting topic that I will be happy to discuss. We do support abbreviations, but they need to be manually added to the lint file. Let's talk about it tomorrow.
There was a problem hiding this comment.
Our coding standards doc (not public) says to use all caps for initialisms.
Not the new docs, which states:
Abbreviations
[Strict] case (e.g. getMeshById, not getMeshByID)
There was a problem hiding this comment.
I don't know what new docs you mean, but abbreviations are not the same as initialisms.
There was a problem hiding this comment.
You are right, but that was what it meant. we can discuss this. Anyhow, abbreviations are supported (and configured). You can see it in the linting file (the first line).
StringPascalCase for example, does not support abbreviations. so It should be HtmlCssSupportId. However, with known and supported abbreviations (or initialisms :-) ) we fallback to CamelCase
packages/dev/core/src/Cameras/Inputs/arcRotateCameraGamepadInput.ts
Outdated
Show resolved
Hide resolved
|
Reviewer - this PR has made changes to one or more package.json files. |
|
Graph tools CI has failed you can find the test results at: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/TOOLS/refs/pull/16455/merge/testResults/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16455/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16455/merge/ |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
Reviewer - this PR has made changes to one or more package.json files. |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/16455/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/16455/merge/ |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
If you have any questions, contact me!
Draft to give everyone time to review and present me with questions.
NOTE - lint currently fails due to javascript out of heap memory. The last few commits are just attempts to get the CI to play nicely. Once I have the solution I will clean the config file and the command running eslint