-
Notifications
You must be signed in to change notification settings - Fork 35
fix(app-data): rename is_omittable property as camelCase in appData
#776
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?
Conversation
it isnt good pattern to use snake case in JSON, so we use camel case instead
📝 WalkthroughWalkthroughAdds app-data v1.13.0: new TypeScript types and JSON schema, bumps latest version constants (app-data and wrappers), exposes v1_13_0 in public exports, adds UTM/Hooks metadata version constants, and updates tests to v1.13.0. Changes
Sequence Diagram(s)(Skipped — changes are schema/type additions and tests without multi-component control-flow changes.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 GitHub Packages PublishedLast updated: Jan 21, 2026, 07:15:25 AM UTC The following packages have been published to GitHub Packages with pre-release version
InstallationThese packages require authentication to install from GitHub Packages. First, create a # Create .npmrc file in your project root
echo "@cowprotocol:registry=https://npm.pkg.github.com" > .npmrc
echo "//npm.pkg.github.com/:_authToken=YOUR_GITHUB_TOKEN" >> .npmrcTo get your GitHub token:
Then install any of the packages above, either by exact version (i.e. # Yarn
yarn add npm:@cowprotocol/cow-sdk@pr-776
# pnpm
pnpm install npm:@cowprotocol/cow-sdk@pr-776
# NPM
npm install npm:@cowprotocol/cow-sdk@pr-776Update to the latest version (only if you used the tag)Every commit will publish a new package. To upgrade to the latest version, run: # Yarn
yarn upgrade @cowprotocol/cow-sdk
# pnpm
pnpm update @cowprotocol/cow-sdk
# NPM
npm update @cowprotocol/cow-sdkView PackagesYou can view the published packages at: https://github.com/cowprotocol/cow-sdk/packages |
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/app-data/test/wrappers.v1.13.0.spec.ts`:
- Line 26: Rename the test title string for the test defined in the
test('Validates valid flashloan metadata v1.13.0', ...) to correctly reflect
what it validates (e.g., "Validates valid wrappers metadata v1.13.0"); update
only the test name in that test call so the function/test identifier remains the
same and CI/test behavior is unchanged.
- Line 14: Rename the test description string in the test call currently written
as 'Creates appDataDoc with flashloan metadata v1.13.0' to accurately reflect
what's being tested (e.g., 'Creates appDataDoc with wrappers metadata v1.13.0')
so the test name matches the wrappers behavior being asserted; update the
test(...) invocation string accordingly in the test file where that literal
appears.
- Line 131: The test title string in the test declaration 'Creates appDataDoc
with flashloan and other metadata v1.13.0' is misleading because the spec
validates wrappers and other metadata, not flashloan; update the test name
inside the test(...) call to accurately describe what's being asserted (for
example, "Creates appDataDoc with wrappers and other metadata v1.13.0") and
ensure any related test descriptions or snapshot names that reference the old
title are updated accordingly so they remain consistent with the assertions in
wrappers.v1.13.0.spec.ts.
| ] | ||
|
|
||
| test('Creates appDataDoc with flashloan metadata v1.11.0', async () => { | ||
| test('Creates appDataDoc with flashloan metadata v1.13.0', async () => { |
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.
Test name mentions "flashloan" but tests wrappers.
The test name 'Creates appDataDoc with flashloan metadata v1.13.0' should reference "wrappers" instead of "flashloan" to match what's being tested.
Suggested fix
- test('Creates appDataDoc with flashloan metadata v1.13.0', async () => {
+ test('Creates appDataDoc with wrappers metadata v1.13.0', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('Creates appDataDoc with flashloan metadata v1.13.0', async () => { | |
| test('Creates appDataDoc with wrappers metadata v1.13.0', async () => { |
🤖 Prompt for AI Agents
In `@packages/app-data/test/wrappers.v1.13.0.spec.ts` at line 14, Rename the test
description string in the test call currently written as 'Creates appDataDoc
with flashloan metadata v1.13.0' to accurately reflect what's being tested
(e.g., 'Creates appDataDoc with wrappers metadata v1.13.0') so the test name
matches the wrappers behavior being asserted; update the test(...) invocation
string accordingly in the test file where that literal appears.
| }) | ||
|
|
||
| test('Validates valid flashloan metadata v1.11.0', async () => { | ||
| test('Validates valid flashloan metadata v1.13.0', async () => { |
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.
Test name mentions "flashloan" but validates wrappers.
Similar to above, this test validates wrappers metadata, not flashloan.
Suggested fix
- test('Validates valid flashloan metadata v1.13.0', async () => {
+ test('Validates valid wrappers metadata v1.13.0', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('Validates valid flashloan metadata v1.13.0', async () => { | |
| test('Validates valid wrappers metadata v1.13.0', async () => { |
🤖 Prompt for AI Agents
In `@packages/app-data/test/wrappers.v1.13.0.spec.ts` at line 26, Rename the test
title string for the test defined in the test('Validates valid flashloan
metadata v1.13.0', ...) to correctly reflect what it validates (e.g., "Validates
valid wrappers metadata v1.13.0"); update only the test name in that test call
so the function/test identifier remains the same and CI/test behavior is
unchanged.
| }) | ||
|
|
||
| test('Creates appDataDoc with flashloan and other metadata v1.11.0', async () => { | ||
| test('Creates appDataDoc with flashloan and other metadata v1.13.0', async () => { |
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.
Test name mentions "flashloan" but tests wrappers with other metadata.
Suggested fix
- test('Creates appDataDoc with flashloan and other metadata v1.13.0', async () => {
+ test('Creates appDataDoc with wrappers and other metadata v1.13.0', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('Creates appDataDoc with flashloan and other metadata v1.13.0', async () => { | |
| test('Creates appDataDoc with wrappers and other metadata v1.13.0', async () => { |
🤖 Prompt for AI Agents
In `@packages/app-data/test/wrappers.v1.13.0.spec.ts` at line 131, The test title
string in the test declaration 'Creates appDataDoc with flashloan and other
metadata v1.13.0' is misleading because the spec validates wrappers and other
metadata, not flashloan; update the test name inside the test(...) call to
accurately describe what's being asserted (for example, "Creates appDataDoc with
wrappers and other metadata v1.13.0") and ensure any related test descriptions
or snapshot names that reference the old title are updated accordingly so they
remain consistent with the assertions in wrappers.v1.13.0.spec.ts.
Description
as commented in the previous PR introducing the wrapper by @alfetopito #746 (comment) , we should be using camel case for all fields in appData. This PR follows up on this previous PR to rename the field as requested.
I also updated the test to explicitly initialize the
isOmittableproperty. I figure the old tests for wrapper hold little value because they won't be used, so I renamed the file.How to Test
Follow README instructions to build the repo and verify no new generated files are included. Additionally, run the tests with
pnpm testinpackages/app-data.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.