feat: LEAP-1341: Build schema.json in create-docs script #6263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's used for autocomplete in config editor. Format is defined by codemirror's plugin `show-hint`.
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
|
/git merge
|
1. Switch to direct run `nx docs editor` instead of `nx run-many`, because we only have `docs` target in editor; it's faster and less confusing 2. Accept path to `schema.json` from env var `SCHEMA_JSON_PATH`: - no hardcoded value, configurable on a correct level - if no var is provided we just don't generate schema.json - this path can also be provided as a 2nd arg for create-docs just in case 3. Apply linting to schema.json, so it won't be rejected by biome check later - biome is applied directly in package.json command, so any usage will have a correct file - `;` after env var set is required to use it in the command in the same line
|
@ellipsis-dev review this |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to a5942b4 in 45 seconds
More details
- Looked at
709lines of code in8files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. web/apps/labelstudio/src/pages/CreateProject/Config/schema.json:292
- Draft comment:
ThetypeforhorizontalAlignmentshould be"string"instead of["left", "center", "right"]. This applies toverticalAlignment,defaultZoom, andcrossOriginas well. Thetypeshould be"string"for these attributes. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The change in the diff to use arrays of specific strings for these attributes seems intentional to enforce stricter validation. The comment suggests reverting this change, which might not align with the intended functionality. The comment does not provide a compelling reason to revert the change, and the new type seems to provide better validation.
I might be missing the context of why the comment suggests reverting to 'string'. It could be due to a misunderstanding of the intended functionality or a lack of clarity in the diff about why the change was made.
The change to use arrays of specific strings is likely to enforce stricter validation, which is generally a good practice. Without a clear reason to revert, the comment seems unnecessary.
The comment should be removed as the change to use arrays of specific strings for these attributes seems intentional and beneficial for validation purposes.
2. web/libs/editor/src/components/Timeline/Views/Frames/Keypoints.tsx:34
- Draft comment:
Typo:firtsPointshould befirstPoint. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is about a change made in the diff, specifically a typo correction. The typo has already been corrected in the PR, so the comment is not necessary anymore. The comment does not suggest any further action or improvement beyond what has already been done.
I might be overlooking the importance of acknowledging typo corrections, but since the typo is already fixed, the comment doesn't add value.
Acknowledging typo corrections is not necessary if the correction is already made in the PR. The focus should be on unresolved issues or improvements.
The comment should be removed because the typo has already been corrected in the PR, making the comment redundant.
3. web/apps/labelstudio/src/pages/CreateProject/Config/schema.json:1259
- Draft comment:
Thetypeforsnapshould be"string"instead of["pixel", "none"]. This applies to other similar attributes as well. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_FFWZ5SeDKP4j349D
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 6 days left in your free trial, upgrade for $20/seat/month or contact us.
yyassi-heartex
approved these changes
Sep 10, 2024
bmartel
approved these changes
Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two goals here:
Add script to generate
schema.jsonused for autocomplete in config editorFormat is defined by codemirror's plugin
show-hint.Path to the file is defined by
SCHEMA_JSON_PATHenv var (see a657eb5)File will be generated in "Build Frontend" CI step every time when we have changes to tag JSDocs.
Fix small issues in a code for new Video timeline spans
Typos, unused styles, outdated comment in docs
PR fulfills these requirements
Describe the reason for change
We need to update autosuggestions in config editor every time we make changes to tags documentation, so they will be in sync.