Skip to content

build: update Dockerfile and deps - #141

Merged
MartinKolarik merged 3 commits into
masterfrom
docker
Jun 18, 2026
Merged

build: update Dockerfile and deps#141
MartinKolarik merged 3 commits into
masterfrom
docker

Conversation

@MartinKolarik

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The Dockerfile is rewritten to use node:22-slim instead of node:4, switching from remote repository cloning to local build context with npm ci --omit=dev and a non-root node user. package.json updates scripts, bumps all dependency versions, and removes chai-http. lib/log.js replaces the le_node logger with a winston-backed wrapper exposing info, warning, err, and on methods. lib/v1.js switches from the minimatch default export to a named makeRe import. tasks/sync.js replaces _.indexBy with _.keyBy. Tests drop chai-http in favor of a custom fetch-based request() helper with a matching expectOkJson() assertion.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether the description is related to the changeset. Add a description explaining the motivation for the dependency updates, any breaking changes, migration notes, or testing performed to validate the updates.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'build: update Dockerfile and deps' is fully related to the changeset, clearly summarizing the main updates to Docker configuration and project dependencies across multiple files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docker

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

@MartinKolarik

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Dockerfile`:
- Line 15: The COPY --chown=node:node . . command in the Dockerfile copies the
entire repository into the image without any filtering, which includes sensitive
and unnecessary files. Create a .dockerignore file at the project root to
exclude: node_modules, .git, .env*, test directories, coverage, and any other
sensitive or bulky paths specific to your project. This file will be
automatically recognized by Docker and prevent these files from being included
in the build context when the COPY command executes.

In `@package.json`:
- Around line 15-25: The parse-env ~0.5 dependency specified in package.json
does not exist on npm and will cause npm ci installation failures. Remove the
parse-env line entirely from the dependencies section, or if environment
variable parsing functionality is needed, replace it with an existing npm
package that provides similar functionality such as dotenv. Verify that all
remaining dependencies in the file can be resolved in the npm registry before
finalizing the fix.

In `@tests/spec/v1-tests.js`:
- Around line 18-26: The fetch response handler unconditionally parses any
non-empty response body as JSON using JSON.parse, which will throw a SyntaxError
if the response contains HTML (such as from a 500 error page), masking the
actual HTTP failure. Guard the JSON.parse call in the then function by checking
if the response content-type header indicates JSON before attempting to parse;
only call JSON.parse when the content-type contains 'application/json',
otherwise return the text as-is or null appropriately to allow test assertions
to see the actual error response.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eda9305b-2f23-4293-af4f-f15aaa3ea117

📥 Commits

Reviewing files that changed from the base of the PR and between ae57062 and 24f5116.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • Dockerfile
  • lib/log.js
  • lib/v1.js
  • package.json
  • tasks/sync.js
  • tests/setup.js
  • tests/spec/v1-tests.js
💤 Files with no reviewable changes (1)
  • tests/setup.js

Comment thread Dockerfile
Comment thread package.json
Comment thread tests/spec/v1-tests.js
@MartinKolarik
MartinKolarik merged commit 2d195b8 into master Jun 18, 2026
1 check 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.

1 participant