Skip to content
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

[#3] Configure API project #14

Conversation

skgndi12
Copy link
Collaborator

@skgndi12 skgndi12 commented Apr 19, 2023

Resolves #3

Purpose

Implement a system that manages config values

Contents

  1. Set entry point
  2. Reflect the JavaScript standard indent rule
  3. Add packages for configuration and seperate packages by environment
  4. Implement a function that assigns config values
    a. Load values from yaml file
    b. Overwritten by env variable if it is exist

@skgndi12 skgndi12 added the enhancement New feature or request label Apr 19, 2023
@skgndi12 skgndi12 self-assigned this Apr 19, 2023
Copy link
Collaborator

@isutare412 isutare412 left a comment

Choose a reason for hiding this comment

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

다음 사항이 필요해보입니다.

  1. Config 에 대한 validation.
  2. 환경변수에 의한 config value overwriting (FOO_SERVER_PORT=8123 이면 file 에서 읽은 config 값을 뒤집어써야 함).
  3. Config validation, Env overwriting 에 대한 유닛 테스트.

@isutare412 isutare412 self-requested a review April 19, 2023 11:19
@skgndi12
Copy link
Collaborator Author

자세한 리뷰 감사합니다. @isutare412

  1. 네 확인했습니다. 추가하도록 하겠습니다.
  2. api/src/config/loader.tsload 함수에 구현된 내용이 있습니다. 수정해야 하는 부분이 있을까요?
  3. 유닛 테스트 부분은 Research unit test TypeScript library #5 의 부분과 겹칠 것 같은데 어떻게 진행할까요?

@isutare412
Copy link
Collaborator

유닛 테스트 부분은 #5 의 부분과 겹칠 것 같은데 어떻게 진행할까요?

유닛 테스트는 그럼 현재 PR 바로 다음 PR로 config loading 및 env overwrite 를 주제로 진행하면 좋을 것 같아요. 괜찮으실까요?

@isutare412 isutare412 self-requested a review April 19, 2023 11:35
@isutare412 isutare412 self-requested a review April 19, 2023 11:37
api/src/main.ts Outdated Show resolved Hide resolved
@isutare412 isutare412 self-requested a review April 19, 2023 11:40
@skgndi12
Copy link
Collaborator Author

skgndi12 commented Apr 20, 2023

유닛 테스트는 그럼 현재 PR 바로 다음 PR로 config loading 및 env overwrite 를 주제로 진행하면 좋을 것 같아요. 괜찮으실까요?

네 현재 PR의 리뷰 반영이 마무리되면 바로 이어서 진행하도록 하겠습니다.

api/.env Outdated Show resolved Hide resolved
@isutare412 isutare412 self-requested a review April 21, 2023 03:32
@skgndi12 skgndi12 force-pushed the feature/issue-3/configure-api-project branch from 12ee3f6 to 1b8ccba Compare April 21, 2023 03:32
@skgndi12
Copy link
Collaborator Author

skgndi12 commented Apr 21, 2023

@isutare412 config 라이브러리를 convict에서 node-config로 변경하였습니다. 관련한 코드 수정 사항은 0c9ab1e 에 있습니다.

Copy link
Collaborator

@isutare412 isutare412 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 👍

@skgndi12
Copy link
Collaborator Author

@isutare412 사용하지 않는 파일 삭제 및 디버깅을 위한 설정값 추가했습니다. 확인 부탁드립니다. 🙇

@skgndi12 skgndi12 force-pushed the feature/issue-3/configure-api-project branch from dc8b4a7 to 15f9475 Compare April 23, 2023 09:49
Copy link
Collaborator

@isutare412 isutare412 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 👍

@skgndi12 skgndi12 merged commit 09f72b8 into MovieReviewComment:develop Apr 23, 2023
@skgndi12 skgndi12 deleted the feature/issue-3/configure-api-project branch April 23, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants