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

코드 리뷰 #1

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

코드 리뷰 #1

wants to merge 2 commits into from

Conversation

juwoong
Copy link
Contributor

@juwoong juwoong commented Jan 7, 2021

  • PEP8 포맷에 맞게 코드 포맷을 변경했습니다.
  • 의미 없는 depth를 제거했습니다
  • 코드에 TODO를 추가했습니다 ( 적절한 주석과 docstring을 요구하는 TODO입니다 )

그와는 별개로, 제출하신 코드에 venv 폴더가 포함되어 있었습니다. 다른 사람에게 파이선 프로젝트를 공유할 때, venv를 그대로 공유하는 것은 절대 하지 말아 주세요. 각 학생별로 환경이 다르기 때문에, 프로젝트의 실행을 보장하기 어렵습니다. 선생님들끼리 상의한 후 수정을 해 주세요. 다음과 같은 선택지가 있습니다.

  1. 직접 venv 만들도록 requirements.txt 파일로 dependency 관리
  2. pipenv 사용하여 dependency 관리
  3. poetry 사용하여 dependency 관리

어쨌거나 학생들이 코드 실행을 위해 기초 설정을 해 줘야 하지만, 그렇게 어려운 것은 아닙니다. 또한 실행하기 위한 명령어를 기록하기 위해 README.md 파일이 존재합니다. 확인 해 보시고 코드 반영해 주세요.

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.

1 participant