-
Notifications
You must be signed in to change notification settings - Fork 12
Update scripts and environment cloud mode #3
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
Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
DockerFiles/Dockerfile.prod
Outdated
COPY . . | ||
|
||
# Compilar o código principal e workflows | ||
# ✅ Injeta o valor fixo antes do build | ||
RUN echo "export const IS_CLOUD_MODE = '${API_CLOUD_MODE}';" > ee/configs/environment.ts |
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.
RUN echo "export const IS_CLOUD_MODE = '${API_CLOUD_MODE//"/\""}';" > ee/configs/environment.ts
The RUN echo
command used to inject environment variables into a TypeScript file is not robust and could fail with special characters.
This issue appears in multiple locations:
- DockerFiles/Dockerfile.prod: Lines 32-32
Please use a more robust method, such as a template file or a build script, to inject environment variables into the TypeScript file.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
src/ee/configs/environment/index.ts
Outdated
* - ✅ In QA/Prod, loads from environment.ts (generated at build time) | ||
*/ | ||
|
||
let environment; |
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.
interface Environment {
// Define expected properties here
}
let environment: Environment;
The environment object structure is not explicitly defined, leading to potential type safety issues.
This issue appears in multiple locations:
- src/ee/configs/environment/index.ts: Lines 10-10
Please add an interface to explicitly define the structure of the environment object for better type safety.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
||
docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG_SHA | ||
docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG_VERSION | ||
docker push $ECR_REGISTRY/$ECR_REPOSITORY:latest |
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.
docker push $ECR_REGISTRY/$ECR_REPOSITORY:latest
docker rmi $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG_SHA $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG_VERSION $ECR_REGISTRY/$ECR_REPOSITORY:latest
The cleanup step to remove local Docker images after pushing to ECR is missing in multiple workflow files.
This issue appears in multiple locations:
- .github/workflows/build-and-push-production.yml: Lines 51-51
- .github/workflows/deploy-to-prod.yml: Lines 75-75
Please add a cleanup step to remove local Docker images after pushing to ECR to free up disk space.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
API_CLOUD_MODE: | ||
(process.env.API_CLOUD_MODE || 'true').toLowerCase() === 'true', |
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.
API_CLOUD_MODE: parseBooleanEnvVar('API_CLOUD_MODE', true),
// In a separate utility file:
function parseBooleanEnvVar(varName: string, defaultValue: boolean): boolean {
const value = process.env[varName]?.toLowerCase();
return ['true', 'false'].includes(value)
? value === 'true'
: defaultValue;
}
The environment variable parsing logic is not centralized, leading to potential maintainability issues.
This issue appears in multiple locations:
- src/ee/configs/environment/environment.dev.ts: Lines 15-16
Please extract the environment variable parsing logic to a separate utility function for better reusability and testability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
import { Environment } from './types'; | ||
|
||
export const environment: Environment = { | ||
API_CLOUD_MODE: __CLOUD_MODE__, |
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.
API_CLOUD_MODE: __CLOUD_MODE__ ?? false,
The CLOUD_MODE variable lacks a default value or validation, which could lead to undefined behavior.
This issue appears in multiple locations:
- src/ee/configs/environment/environment.template.ts: Lines 6-6
- src/ee/configs/environment/environment.template.ts: Lines 6-6
Please add a default value or validation for CLOUD_MODE to prevent undefined behavior.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
No description provided.