Skip to content

misc minor changes#377

Merged
vicb merged 2 commits intomasterfrom
vicb/misc-20250918
Sep 18, 2025
Merged

misc minor changes#377
vicb merged 2 commits intomasterfrom
vicb/misc-20250918

Conversation

@vicb
Copy link
Copy Markdown
Owner

@vicb vicb commented Sep 18, 2025

Use Node 24 for the fecther
connect-redis v9 does not support ioredis any more

Summary by Sourcery

Upgrade the fetcher environment to Node 24 and revert connect-redis to v8 for ioredis support.

Enhancements:

  • Pin connect-redis to version 8.x to retain ioredis compatibility

Build:

  • Use the Node current-slim (Node 24) image for the fetcher Dockerfile

Summary by CodeRabbit

  • Chores
    • Changed the container base image for the fetcher service to a newer Node.js slim variant.
    • Updated the Redis session store dependency to the 8.x line.

No user-facing functionality changes are expected. These updates align the runtime environment and dependencies, improving consistency across services and ensuring stable builds without altering application behavior.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Sep 18, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR updates the fetcher service to run on Node 24 by switching its Docker base image and restores ioredis support by downgrading connect-redis to version 8, with corresponding updates to the lockfile.

File-Level Changes

Change Details Files
Bump fetcher service Node version
  • Replaced Docker base image from node:lts-slim to node:current-slim
apps/fetcher/Dockerfile
Revert connect-redis to v8 for ioredis compatibility
  • Changed connect-redis dependency from ^9.0.0 to 8.x in package.json
  • Regenerated package-lock.json to lock v8 dependency
package.json
package-lock.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Sep 18, 2025

Walkthrough

The Docker base image for apps/fetcher changed from node:lts-slim to node:current-slim. The package.json dependency for connect-redis was updated from ^9.0.0 to 8.x. No other instructions or configurations were modified.

Changes

Cohort / File(s) Summary
Docker image update
apps/fetcher/Dockerfile
Switch base image from docker.io/node:lts-slim to docker.io/node:current-slim; other instructions unchanged.
Dependency version change
package.json
Downgrade connect-redis dependency from ^9.0.0 to 8.x.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Use the node LTS release #302 — Also changes apps/fetcher/Dockerfile’s Node base image, but in the opposite direction (current → lts), indicating a directly related discussion on Node version choice.

Poem

In a hop and a skip, I tweak my cart,
From LTS burrow to Current’s start.
Redis friend steps back a pace,
Eight.x keeps a steady pace.
Carrots compiled, caches fed—
Ship the crate, then back to bed. 🥕🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title "misc minor changes" is too vague and does not summarize the PR's primary changes (the fetcher Dockerfile base image update to Node 24 and the connect-redis dependency change from ^9.0.0 to 8.x), so it does not provide clear context for reviewers. Please rename the PR to a concise, descriptive title such as "fetcher: upgrade to Node 24 and pin connect-redis to 8.x" and add a one-line reason in the description (e.g., ioredis compatibility) so reviewers can immediately see the intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vicb/misc-20250918

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying flyxc with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0e252f2
Status: ✅  Deploy successful!
Preview URL: https://9cafa49d.flyxc.pages.dev
Branch Preview URL: https://vicb-misc-20250918.flyxc.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5bfa77d and 0e252f2.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • apps/fetcher/Dockerfile (1 hunks)
  • package.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Sourcery review
  • GitHub Check: Analyze (javascript)
  • GitHub Check: build (22.x)
🔇 Additional comments (1)
package.json (1)

119-119: Pin connect-redis to a concrete range

  • package.json (line 119) currently: "connect-redis": "8.x" — atypical and still floating; change to one of:
    Option A (caret):
    -    "connect-redis": "8.x",
    +    "connect-redis": "^8.0.0",
    Option B (exact; use the CI-validated version):
    -    "connect-redis": "8.x",
    +    "connect-redis": "8.X.Y",
  • Repo search found no connect-redis/ioredis usages — verify which exact version your CI tests and confirm Redis store initialization (constructor signature and typings) remains compatible with ioredis.

# Use the official lightweight Node.js image.
# https://hub.docker.com/_/node
FROM docker.io/node:lts-slim
FROM docker.io/node:current-slim
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Pin Node to 24 for reproducible builds (avoid floating current).

Use a fixed base to match the PR goal “use Node 24” and prevent surprise major upgrades.

Apply:

-FROM docker.io/node:current-slim
+FROM docker.io/node:24-slim

Optional hardening and image hygiene:

-RUN apt-get update -y \
-	&& apt-get install -y tini
+RUN apt-get update -y \
+	&& apt-get install -y --no-install-recommends tini \
+	&& rm -rf /var/lib/apt/lists/*

Consider removing the non-deterministic global npm upgrade:

-RUN npm install -g npm@latest
+# Rely on image's npm (or pin a specific major if required)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/fetcher/Dockerfile around line 3, the base image is using the floating
tag "node:current-slim"; change it to a fixed Node 24 image (for example
"node:24-slim" or a specific digest) to ensure reproducible builds, remove or
avoid any global non-deterministic npm upgrade commands in the Dockerfile, and
optionally apply image-hardening best practices such as switching to a non-root
user, minimizing layers, and pinning package manager versions or using a digest
for the base image.

@vicb vicb merged commit 0633d6b into master Sep 18, 2025
7 checks passed
@vicb vicb deleted the vicb/misc-20250918 branch September 18, 2025 06:47
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.

1 participant