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

refactor : replace Resp interface to models #38

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

zcon-cychoi
Copy link
Contributor

@zcon-cychoi zcon-cychoi commented Dec 6, 2023

  1. models 패키지 추가
  2. Response map[string]interface{}modelsstruct 타입으로 변경
  3. 마이그레이션 관련 페이지 핸들러들을 controllerspageHandlers.go 로 이동

Copy link
Member

@yunkon-kim yunkon-kim left a comment

Choose a reason for hiding this comment

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

@zcon-cychoi

generatedHandlers.go 상단의 type BasicResponse struct {}basicModels.gotype BasicPostResponse struct {} 하나로 통합하는 것이 의미상으로 맞을 것 같습니다.

  • 기존 generatedHandlers.go 상단의 type BasicResponse struct {} 는 삭체 처리 해주시고요.
  • basicModels.gotype BasicPostResponse struct {} 구조체 명을 BasicResponse로 변경 바랍니다.

@yunkon-kim yunkon-kim linked an issue Dec 8, 2023 that may be closed by this pull request
@yunkon-kim
Copy link
Member

@zcon-cychoi

아래 사항은 별도로 진행중이실까요? PR 타이틀 상으로 Request Body를 포함하는 것 같아 문의드리는 부분입니다 ^^

(Ref. #35 코멘트)

@zcon-cychoi 넵 설명 감사드립니다. 자세히 설명해 주셔서 바로바로 이해가 잘 되네요 😃

Migration 이외의 파트를 제가 구분하여 PR 올리도록 하겠습니다.

Migration 관련 파트는 Credential 관련 부분을 분리해야할 필요가 있고요.
아래 두 가지 안 정도를 생각해 볼 수 있을 것 같습니다.
1안) cm-data-mold binary와 같은 디렉토리에 credential을 사전 세팅
2안) Credential을 최초 한번 업로드하고, 이후 Migration시 file의 ID 또는 path를 활용하는 방안

관련하여 수정 가능여부, 공수, 이슈 등을 먼저 파악해 보려고 합니다. 확인해 보시고 공유해주시기 바랍니다.
(코드 개선은 이후에 진행하도록 하겠습니다.)

@zcon-cychoi
Copy link
Contributor Author

@yunkon-kim request 부분은 수정하지 않았네요. 혼선을 드려 죄송합니다. crendential 부분은 따로 진행해보겠습니다

@yunkon-kim
Copy link
Member

@zcon-cychoi 넵! 현황 및 계획을 공유해주셔서 감사드립니다!

그럼 이 PR에서는 리뷰 코멘트 사항에 대해서만 다루도록 하겠습니다 ^^

@zcon-cychoi zcon-cychoi changed the title refactor : replace Req/Resp interface to models refactor : replace Resp interface to models Dec 12, 2023
Copy link
Member

@yunkon-kim yunkon-kim left a comment

Choose a reason for hiding this comment

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

@zcon-cychoi LGTM!

@yunkon-kim yunkon-kim merged commit 80dc210 into cloud-barista:main Dec 12, 2023
2 checks passed
@zcon-cychoi zcon-cychoi deleted the model-patch branch December 12, 2023 02:02
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.

Improvements to provide REST API and documentation
2 participants