Skip to content
This repository has been archived by the owner on Sep 30, 2019. It is now read-only.

프로젝트 기본 구조 설정 #1

Merged
merged 21 commits into from
Mar 12, 2019
Merged

프로젝트 기본 구조 설정 #1

merged 21 commits into from
Mar 12, 2019

Conversation

rscarrera27
Copy link
Contributor

@rscarrera27 rscarrera27 commented Mar 9, 2019

프로젝트 기본 구조를 셋업 했습니다.
커넥션, 디스크립터와 모델쪽을 중점으로 보시면 됩니다

라우터는 임시 플레이스 홀더라서 보실 필요 없습니다
디스크립터에는 조금 메타클래스 적용한 부분이 있어서 사전 지식이 있으시면 좋지만 큰 부분은 아닙니다

@rscarrera27 rscarrera27 requested review from f1nn2, LumpKim, Gaegul and a team March 9, 2019 13:51
Copy link

@f1nn2 f1nn2 left a comment

Choose a reason for hiding this comment

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

PR 자체 규모가 커서 디테일한 리뷰가 조금 힘들었던 것 같습니다.
커밋 단위로 변경사항을 알려주시거나 규모를 줄이는 것도 좋을 것 같습니다. 단순히 구조를 세팅하는 PR 이어서 이번에 디테일하게 하지 못한 리뷰는 이후 코드리뷰를 위해 트래킹하면서 진행하겠습니다.

  • 매직 밸류가 너무 많아 어느정도 모듈 상수가 필요해 보입니다.

user/app.py Outdated Show resolved Hide resolved
user/descriptor.py Outdated Show resolved Hide resolved
user/descriptor.py Outdated Show resolved Hide resolved
user/model.py Outdated
"name_index": ("admin_name", ),
}

table_creation_statement = f"""
Copy link

Choose a reason for hiding this comment

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

ORM 과 pure query 를 함께 사용한 이유는 무엇인가요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ORM을 사용한 부분이 없습니다! 혹시 ORM 이라고 생각하신 부분이 무엇인가요?

@rscarrera27 rscarrera27 requested a review from f1nn2 March 12, 2019 00:21
Copy link

@LumpKim LumpKim left a comment

Choose a reason for hiding this comment

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

좋은 것 같아요

@Gaegul
Copy link

Gaegul commented Mar 12, 2019

코드 읽어 보았습니다. user 디렉토리 안에 대부분의 파일이 있던데 왜 그런지 여쭤봐도 되나요?

Copy link

@LumpKim LumpKim left a comment

Choose a reason for hiding this comment

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

좋은 것 같아요

@rscarrera27
Copy link
Contributor Author

@Gaegul 원래 서비스 이름이 user 여서 user란 이름으로 루트 디렉토리가 잡혀있습니다. 변경하도록 하겠습니다

@rscarrera27 rscarrera27 merged commit afe731a into master Mar 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants