-
Notifications
You must be signed in to change notification settings - Fork 26
fix: filter out new keys from legacy JSON generator #595
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #595 +/- ##
==========================================
- Coverage 80.80% 80.60% -0.21%
==========================================
Files 126 126
Lines 12073 12132 +59
Branches 874 874
==========================================
+ Hits 9756 9779 +23
- Misses 2314 2350 +36
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Removed commented TODO note about additional keys in JSON output.
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 updates the legacy-json generator to emit “legacy-compatible” JSON by filtering the serialized output to a specific allowlist of keys, aiming to prevent newly introduced fields from appearing in generated legacy JSON files.
Changes:
- Replaces
JSON.stringify(section, null, 2)withJSON.stringify(section, [allowedKeys...], 2)to filter output keys. - Conditionally includes the
moduleskey for non-indexsections during serialization.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| File | Base | Head | Diff |
|---|---|---|---|
orama-db.json |
8.03 MB | 8.03 MB | -3.00 B (-0.00%) |
avivkeller
left a comment
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.
ovflowd
left a comment
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.
Let's wait for the TSC meeting.
|
To be clear, not against the PR, just want to want for the TSC meeting before we make any decisions on the approach. |
6a3906b to
236c8b0
Compare
ovflowd
left a comment
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.
Actually, let's land this, I can't really see negatives of why not landing this.
Description
Validation
Related Issues
Check List
node --run testand all tests passed.node --run format&node --run lint.