-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: add support for node-20 #71
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe updates span various configuration files and codebases to better align with newer Node.js versions and enhance overall system robustness. Core changes include upgrading Node.js from 14.17.x to 20.15.0 across deployment, CI, and Docker environments, modifying Changes
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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: 1
Outside diff range and nitpick comments (2)
docker-compose.yml (2)
Line range hint
7-7
: Fix indentation.The indentation is incorrect. Expected 8 but found 10.
- ENVIRONMENT_NAME: ${ENVIRONMENT_NAME} + ENVIRONMENT_NAME: ${ENVIRONMENT_NAME}
Line range hint
31-31
: Add newline at the end of the file.A newline character is missing at the end of the file.
- command: ["redis-server", "--bind", "redis", "--port", "6379"] + command: ["redis-server", "--bind", "redis", "--port", "6379"] +
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (13)
- .github/workflows/cd.yml (1 hunks)
- .github/workflows/ci.yml (1 hunks)
- .github/workflows/coverage-report-ci.yml (1 hunks)
- .gitignore (1 hunks)
- Dockerfile (1 hunks)
- README.md (1 hunks)
- tests/server/api/cronJob/aggregateJob.test.js (2 hunks)
- tests/server/middlewares/auth/index.test.js (2 hunks)
- babel.config.js (1 hunks)
- docker-compose.yml (1 hunks)
- package.json (4 hunks)
- server/database/models/storeProducts.js (1 hunks)
- server/database/models/supplierProducts.js (1 hunks)
Files skipped from review due to trivial changes (6)
- .github/workflows/ci.yml
- .github/workflows/coverage-report-ci.yml
- .gitignore
- tests/server/api/cronJob/aggregateJob.test.js
- tests/server/middlewares/auth/index.test.js
- babel.config.js
Additional context used
Hadolint
Dockerfile
[error] 5-5: Use COPY instead of ADD for files and folders
(DL3020)
[error] 16-16: Use COPY instead of ADD for files and folders
(DL3020)
[error] 17-17: Use COPY instead of ADD for files and folders
(DL3020)
yamllint
docker-compose.yml
[warning] 7-7: wrong indentation: expected 8 but found 10
(indentation)
[error] 31-31: no new line character at the end of file
(new-line-at-end-of-file)
LanguageTool
README.md
[style] ~35-~35: Consider a different adverb to strengthen your wording.
Context: ...4"> --- We’re always looking for people who value their work...(ALWAYS_CONSTANTLY)
[uncategorized] ~124-~124: A comma might be missing here.
Context: ...ocumentation Once you've to the server started check out the api documentation at [/ap...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~144-~144: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...compose](./docker-compose.yml) - [auto generated apis](./server/api/requestGenerators.js...(AUTO_HYPHEN)
[typographical] ~150-~150: After the expression ‘for example’ a comma is usually used.
Context: ...e multiple copies of the same data. For example - Orders contains purchasedProducts ...(COMMA_FOR_EXAMPLE)
[uncategorized] ~152-~152: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... the same data. For example - Orders contains purchasedProducts which contains Produc...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~152-~152: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...Orders contains purchasedProducts which contains Products. Instead of referencing here w...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~156-~156: A comma might be missing here.
Context: ...eavy. Every time there is a change to a product we need to make a change to - Suppli...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~162-~162: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: ... - StoreProducts - Products Orders is not impacted since a change in the prod...(AI_HYDRA_LEO_CPT_IS_ARE)
[uncategorized] ~163-~163: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...er purchase will not affect the order. However the application is able to perform extr...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~164-~164: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...ect the order. However the application is able to perform extremely fast reads. 2 reasons...(BE_ABLE_TO)
[typographical] ~169-~169: It appears that a comma is missing.
Context: ... to its ability to have shards. In this application we create 4 shards and the data is dist...(DURING_THAT_TIME_COMMA)
[style] ~169-~169: The preposition ‘amongst’ is correct, but some people think that it is old-fashioned or literary. A more frequently used alternative is the preposition “among”.
Context: ...te 4 shards and the data is distributed amongst these shards. These are the shard keys...(AMONGST)
[style] ~179-~179: Consider using an extreme adjective for ‘good’.
Context: ...iers - Stores
We got really good distribution across shards(24-26%) per ...(EXTREME_ADJECTIVES)
[uncategorized] ~179-~179: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... possible to get a hot shard due to this but we're yet to see that. - productId ...(COMMA_COMPOUND_SENTENCE)
Markdownlint
README.md
132-132: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
133-133: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
135-135: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
136-136: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
137-137: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
139-139: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
140-140: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
141-141: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
142-142: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
143-143: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
144-144: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
145-145: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
146-146: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
174-174: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
176-176: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
177-177: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
178-178: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
181-181: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
182-182: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
41-41: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
88-88: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
87-87: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
98-98: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
108-108: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
1-1: null
Images should have alternate text (alt text)(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
26-26: null
Images should have alternate text (alt text)(MD045, no-alt-text)
29-29: null
Images should have alternate text (alt text)(MD045, no-alt-text)
51-51: null
Images should have alternate text (alt text)(MD045, no-alt-text)
52-52: null
Images should have alternate text (alt text)(MD045, no-alt-text)
55-55: null
Images should have alternate text (alt text)(MD045, no-alt-text)
56-56: null
Images should have alternate text (alt text)(MD045, no-alt-text)
98-98: Expected: indented; Actual: fenced
Code block style(MD046, code-block-style)
108-108: Expected: indented; Actual: fenced
Code block style(MD046, code-block-style)
Additional comments not posted (15)
server/database/models/storeProducts.js (2)
6-6
: Change tomongoose.ObjectId
is appropriate.The change from
mongoose.Types.ObjectId
tomongoose.ObjectId
is correct and ensures consistency with the rest of the codebase.
10-10
: Change tomongoose.ObjectId
is appropriate.The change from
mongoose.Types.ObjectId
tomongoose.ObjectId
is correct and ensures consistency with the rest of the codebase.Dockerfile (3)
1-1
: Node.js version upgrade to 20-alpine is appropriate.The upgrade to Node.js version 20 is correct and aligns with the PR objectives.
9-9
: Dynamic build command is appropriate.The change to dynamically use the environment name in the build command is correct and ensures flexibility in the build process.
15-15
: Addition ofbull
package is appropriate.The addition of the
bull
package is correct and aligns with the requirements.server/database/models/supplierProducts.js (2)
6-6
: Change tomongoose.ObjectId
is appropriate.The change from
mongoose.Types.ObjectId
tomongoose.ObjectId
is correct and ensures consistency with the rest of the codebase.
10-10
: Change tomongoose.ObjectId
is appropriate.The change from
mongoose.Types.ObjectId
tomongoose.ObjectId
is correct and ensures consistency with the rest of the codebase.docker-compose.yml (1)
12-12
: Reference to.env.docker
is appropriate.The change to reference
.env.docker
instead of.env.local
is correct and aligns with the updated environment configuration..github/workflows/cd.yml (1)
59-59
: Update Node.js version to 20.15.0The Node.js version has been updated to 20.15.0. Ensure that all dependencies and scripts are compatible with this version.
package.json (5)
7-7
: Update test scriptThe test script has been updated to include additional flags for Jest. Ensure that these flags are necessary and compatible with your test setup.
10-11
: Add new build commandsNew build commands for Docker and local environments have been added. Verify that these commands work as expected in your build process.
44-44
: Update Node.js version to 20.15.xThe Node.js version has been updated to 20.15.x. Ensure that all dependencies and scripts are compatible with this version.
74-74
: Update opossum dependencyThe
opossum
dependency has been updated to version 8.1.4. Ensure that this version is compatible with your codebase.
84-97
: Add new Babel and Jest dependenciesNew Babel and Jest dependencies have been added. Verify that these dependencies are necessary and compatible with your codebase.
Also applies to: 102-102
README.md (1)
90-92
: Update Docker commands to use .env.dockerThe Docker commands have been updated to use
.env.docker
. Ensure that this environment file contains the necessary configurations for your Docker setup.
@@ -1,17 +1,18 @@ | |||
FROM node:14.17-alpine AS build1 | |||
FROM node:20-alpine AS build1 | |||
ARG ENVIRONMENT_NAME | |||
ENV ENVIRONMENT_NAME $ENVIRONMENT_NAME | |||
RUN mkdir -p /app-build | |||
ADD . /app-build |
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.
Use COPY
instead of ADD
for files and folders.
According to hadolint, COPY
should be used instead of ADD
for files and folders.
- ADD . /app-build
+ COPY . /app-build
- ADD package.json /
- ADD . /
+ COPY package.json /
+ COPY . /
Also applies to: 16-17
Tools
Hadolint
[error] 5-5: Use COPY instead of ADD for files and folders
(DL3020)
Ticket Link
Related Links
Description
Steps to Reproduce / Test
Checklist
yarn test
passesGIF's
Summary by CodeRabbit
Chores
.gitignore
to excludereport.json
and include.env.docker
.docker-compose.yml
to.env.docker
.Documentation
README.md
to reflect newdocker-compose
commands using.env.docker
.Tests
aggregateJob.test.js
andindex.test.js
.Refactor
babel.config.js
with new presets and plugins.New Features
package.json
.bull
package in Dockerfile.