Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
=======================================
Coverage 86.87% 86.87%
=======================================
Files 503 503
Lines 25660 25660
Branches 2630 2630
=======================================
Hits 22292 22292
Misses 3045 3045
Partials 323 323 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| import { PackageManagers } from "starlight-package-managers"; | ||
|
|
||
| Generate diagnostics for content complexity using `@player-tools/complexity-check-plugin`. The plugin analyzes your content structure and calculates complexity based on asset types, nesting depth, and overall structure to provide a single complexity score per file. |
There was a problem hiding this comment.
-
Would it be clearer to say "DSL content" or "JSON content"? Or even just capitalize "Content"?
When I initially read this, I didn't understand what this plugin did at all. I think I read "content" as the general use of the word instead of Player Content. We use "content" a lot in the player space, despite it actually being a very ambiguous and common word. I think that can be confusing. But it's a pattern we have established so I leave whether to change it up to you.
-
For my own knowledge / not a review question: what is the purpose of the complexity check plugin? Are we trying to get consumers to limit complexity to less than a certain number?
There was a problem hiding this comment.
FWIW I would go with Player Content. Its worth making a distinction when it comes to how its authored, but in general content (or player content) is generally what the input data for Player is referred to.
There was a problem hiding this comment.
I'll update to Player Content 👍 .
@KVSRoyal there is a correlation between complexity score and time to render - so the higher the complexity, the worse it is because there's a lot to parse/run-time implications. So the warning/error numbers are to help identify which files could stand to be broken up into smaller parts. Because the plugin provides these complexity scores, we can use them in other utils like comparing PR changes to base branches, etc.
|
|
||
| Install the plugin: | ||
|
|
||
| <PackageManagers pkg="@player-tools/complexity-check-plugin" /> |
There was a problem hiding this comment.
This is cool. Didn't know we could use this.
| ### Configuration Options | ||
|
|
||
| - **`maxAcceptableComplexity`**: Maximum complexity before error (default: 10000) | ||
| - **`maxWarningLevel`**: Complexity level that triggers warnings (default: 1000) | ||
| - **`typeWeights`**: Custom complexity weights for different asset types | ||
| - **`baseWeightOverrides`**: Override base complexity weights |
There was a problem hiding this comment.
Do you think we could leverage some docs auto-generator to transfer code docs into docs here? These types of things seem tedious to maintain and it's easier to put more detail if we only have to do it in one place.
Absolutely free to make this suggestion into a future ticket instead of doing it now. I don't expect you to redo a non-trivial part of how our docs work in this PR. 😅
There was a problem hiding this comment.
100% yes. That's an issue I keep running into where the README of the source package has a lot of this and we're just repeating the same content. I'll make an issue to investigate streamlining this better.
|
|
||
| import { PackageManagers } from "starlight-package-managers"; | ||
|
|
||
| Generate a JSON file of metrics using `@player-tools/metrics-output-plugin`. The plugin consumes diagnostics from LSP plugins and produces a single JSON file with per-file metrics and optional project-level metadata. |
There was a problem hiding this comment.
What does "LSP" stand for here?
There was a problem hiding this comment.
Oh. Okay, can we link out to that? I don't think that's a common acronym 😅
There was a problem hiding this comment.
Linking out seems excessive to me. If someone is reading about LSP/language plugins its not an unreasonable assumption that they are aware of what an LSP is. At the minimum though, I think it would be fine to our internal page on how our LSP is architected.
There was a problem hiding this comment.
For now, I think since both of these plugins are under the language section, that it is a little self explanatory. That being said, I think having a more robust Plugin Overview page with glossary terms and additional context would be good.
| @@ -21,153 +21,249 @@ You can do this by installing the React Player and Reference Assets Packages | |||
| </Fragment> | |||
There was a problem hiding this comment.
Is this stuff and architecture.mdx just copied over from old docs?
There was a problem hiding this comment.
Yep - definitely need some cleanup/updates here. Although, oops - didn't mean to include architecture; thanks for asking about it
Fixing redirects and adding docs around tooling, specifically complexity + metrics LSP plugins, restoring getting-started content.
Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ADoes your PR have any documentation updates?