Skip to content

fix: backward compatibility for puter kv incr/decr#1697

Merged
Salazareo merged 1 commit intomainfrom
DS/main
Oct 7, 2025
Merged

fix: backward compatibility for puter kv incr/decr#1697
Salazareo merged 1 commit intomainfrom
DS/main

Conversation

@Salazareo
Copy link
Copy Markdown
Member

No description provided.

Comment thread src/puter-js/index.d.ts Outdated
Comment on lines +300 to +301
incr(key: string, pathAndAmount?: { [key: string]: number } | number): Promise<number>;
decr(key: string, pathAndAmount?: { [key: string]: number } | number): Promise<number>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should we do multi types or method overloading with different types?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw this is just me wanting to know more about the decision 😓
i don't know or have preference for either

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah can update

const getRes = await puter.kv.get(TEST_KEY);
expect(getRes).toBe(0);
await puter.kv.set(TEST_KEY, 0);
const incrRes = await puter.kv.incr(TEST_KEY, { '': 5 });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for 'should increment a key success' do we intend to have the argument? since other test below already include the argument
i wonder if this test is for without the argument

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes will rename

const getRes = await puter.kv.get(TEST_KEY);
expect(getRes).toBe(5);
const getRes = await puter.kv.set(TEST_KEY, 0);
const decrRes = await puter.kv.decr(TEST_KEY, { '': 3 });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, i wonder if this test is for the one without pathAndAmount param


it('should increment a key with second argument', async () => {
await puter.kv.set(TEST_KEY, 0);
const incrRes = await puter.kv.incr(TEST_KEY);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this test is missing the second argument


it('should increment a key with second argument', async () => {
await puter.kv.set(TEST_KEY, 0);
const incrRes = await puter.kv.decr(TEST_KEY);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this test is missing the second argument

@reynaldichernando
Copy link
Copy Markdown
Member

other than the test cases, i think this is good to go 🚀

@Salazareo Salazareo force-pushed the DS/main branch 2 times, most recently from 06a3951 to f0bfffb Compare October 7, 2025 18:37
@Salazareo Salazareo merged commit 2a2a1f6 into main Oct 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants