-
Notifications
You must be signed in to change notification settings - Fork 12
Rename cardDef fields #3800
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
base: main
Are you sure you want to change the base?
Rename cardDef fields #3800
Conversation
Host Test Results 1 files ±0 1 suites ±0 1h 29m 26s ⏱️ -39s For more details on these failures and errors, see this check. Results for commit 2e06ece. ± Comparison against base commit b2f9b85. This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.This pull request skips 2 tests.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR renames several field names in the CardDef schema for improved clarity and consistency. The primary change is renaming generic field names like title, description, and thumbnailURL to more specific names prefixed with "card" to better distinguish them from other similar fields.
Key changes:
- Renamed
title→cardTitle - Renamed
description→cardDescription - Renamed
thumbnailURL→cardThumbnailURL
Reviewed changes
Copilot reviewed 299 out of 2009 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| JSON configuration files (catalog-realm/*) | Updated field names in card instance data across hundreds of card definitions |
| TypeScript/GTS template files (base/*) | Updated field references in card class definitions and component templates |
| 3d-model-viewer files | Updated field names in GLTF viewer component and sample data |
| boxel-motion test files | Updated field references in test component templates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot can you review all 2000+ files? |
|
@jurgenwerk I've opened a new pull request, #3801, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@FadhlanR Is there a plan for card JSON in realms? |
Do you mean the user realms in staging and production? We either need a script to check each JSON file and update them accordingly, or we can ignore them, leaving the new fields with null value. |
|
Yes user realms on staging, prod and local dev |
Preview deployments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44e6ab409
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@FadhlanR have you verified that our server environment have a python install capable of running this script? |
|
@lukemelia here is the node version, I've tested the |
|
@FadhlanR all of us dev will also need to run this script. can we just add it to a scripts folder in this repo so that it is easy for us to migrate our own envs. make sure to document clearly what we need to do in our own envs after we merge upstream into our envs. |
|
Also I imagine we need to abandon any AI conversations that might have old card structures in them. we should document that as well. |
Rename
title,description,thumbnailURLto be:cardTitle,cardDescription,cardThumbnailURLRename
cardInfo.title,cardInfo.descriptionto be:cardInfo.name,cardInfo.summaryHere is the script to rename the fields in the JSON files in staging and production, I've tested to rename the boxel skills card instances (cardstack/boxel-skills#43).