Skip to content

Merge pull requests 33 and 28 from mrjasonroy's repo.#34

Closed
jellylamp wants to merge 4 commits into
mrjasonroy:mainfrom
Modea:mergePRs
Closed

Merge pull requests 33 and 28 from mrjasonroy's repo.#34
jellylamp wants to merge 4 commits into
mrjasonroy:mainfrom
Modea:mergePRs

Conversation

@jellylamp

Copy link
Copy Markdown

No description provided.

SerMedvid and others added 4 commits February 24, 2026 11:19
The RedisClient interface now uses node-redis style SET options
({ EX: seconds }). The factory adapter must translate these to
ioredis's positional format ("EX", seconds).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jellylamp jellylamp requested a review from mrjasonroy as a code owner June 15, 2026 19:08
@jellylamp jellylamp closed this Jun 15, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Redis cache handler to use node-redis style options ({ EX: seconds }) instead of ioredis positional arguments, updating the adapter, client interface, and tests accordingly. It also adds end-to-end testing setup instructions and scripts. Feedback includes correcting markdown list formatting in CONTRIBUTING.md and adding a defensive check in redis.ts to prevent Redis errors when the calculated TTL is non-positive.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread CONTRIBUTING.md
Comment on lines +23 to +30
4. **Setup e2e testing**:
```bash
pnpm setup:e2e
```
5. **Create a branch** for your work:
```bash
git checkout -b feature/my-new-feature
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The markdown code blocks for steps 4 and 5 are not indented, which breaks the list formatting in many markdown renderers. Indenting the code blocks by 3 spaces keeps them nested within their respective list items:

4. **Setup e2e testing**:
   ```bash
   pnpm setup:e2e
  1. Create a branch for your work:
    git checkout -b feature/my-new-feature

Comment on lines 277 to +278
// Store in Redis with TTL
await redis.set(key, JSON.stringify(serialized), "EX", Math.ceil(ttl));
await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If ttl is calculated to be 0 or negative (for example, if entry.expire is 0 or negative), passing it to Redis EX option will result in a Redis error (ERR invalid expire time in set). Adding a defensive check to skip caching when ttl <= 0 prevents unnecessary Redis errors.

        if (ttl <= 0) {
          log?.("set", cacheKey, "skipped due to non-positive TTL", { ttl });
          return;
        }

        // Store in Redis with TTL
        await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) });

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.

4 participants