Conversation
haonkr
force-pushed
the
team-1
branch
2 times, most recently
from
aede537 to
13207e5
Compare
dohun0310
requested review from
Donghwan814,
dohun0310,
leesj0706 and
seojoon1
and removed request for
127lucas,
LEEMINGU03,
haonkr and
ohsanghy09
|
안녕하세요, 소영님! 이번 리뷰는 코드의 정답 여부를 평가하기보다는, 이번 과제를 통해 Git과 GitHub에 한층 더 익숙해지셨기를 바랍니다 :) |
dohun0310
left a comment
dohun0310
left a comment
There was a problem hiding this comment.
먼저, PR 작성 방식에 대해 여쭤보고 싶은데요.
팀에서 Notion으로 정리해주신 문서를 확인했는데,
PR 양식에 대한 기준은 따로 명시되어 있지 않은 것 같았습니다.
이번 PR을 작성하실 때,
팀원분들과 PR 작성 방식에 대해 따로 논의하신 부분이 있었을까요?
또, 현재 PR이 ‘주요 작업 내용’과 ‘관련 이슈’ 중심으로 구성되어 있는데,
이와 같은 형태로 작성하신 이유나 기준이 있으셨는지도 함께 궁금합니다.
dohun0310
left a comment
dohun0310
left a comment
There was a problem hiding this comment.
사용하신 단어에 대해서도 여쭤보고 싶어요.
브랜치 이름이 Feat/main#7이고, PR과 Issue 모두 메인 기능이라고 명시해주셨는데요.
이 프로젝트에서 메인 기능이 무엇인지 여쭤봐도 될까요?
제가 생각하는 계산기의 메인 기능은 덧셈, 뺄셈, 곱셈, 나눗셈 같은 연산 기능인데,
소영님이 이번에 작업하신 내용은 import, 재실행과 같은 로직입니다.
그럼에도 메인 기능이라고 표현하신 이유가 궁금합니다.
또 한 가지, 브랜치 이름에 main이 들어가 있다 보니
Git의 main 브랜치와 관련된 작업인지 헷갈리기도 했어요.
브랜치 이름은 해당 작업의 내용을 팀원에게 전달하는 역할도 하는데,
이 이름을 선택하신 기준이 따로 있으셨는지 궁금합니다.
dohun0310
left a comment
dohun0310
left a comment
There was a problem hiding this comment.
작성하신 Issue #7과 이번 PR의 내용이 제 눈에는 별다른 차이가 없어 보이는데요.
혹시 Issue와 PR을 각각 어떤 용도의 공간이라고 생각하시는지 여쭤봐도 될까요?
다른 오픈소스 프로젝트나 팀들은 이 두 공간을 어떻게 구분해서 작성하는지,
그리고 왜 그렇게 쓰는지에 대해서도 한번 찾아보시면 좋을 것 같아요.
이번 제출에서는 두 내용이 같다고 느껴졌는데,
의도적으로 같게 작성하신 건지,
아니면 두 공간의 차이가 아직 잘 와닿지 않으셨던 건지 궁금합니다!
There was a problem hiding this comment.
Assignees 설정에 관해서도 이야기 나눠보고 싶어요.
Issue와 PR 모두 팀원 전원이 Assignees로 지정되어 있는 걸 확인했는데, 이렇게 설정하신 이유가 궁금합니다.
Assignees에 누구를 지정해야 하는지, 그리고 왜 지정하는지에 대해 어떻게 이해하고 계신지 여쭤봐도 될까요?
또, 모든 Issue와 PR에 팀원 전원을 Assignees로 지정하는 것이 팀 내에서 논의를 거쳐 정해진 규칙인지도 궁금합니다.
저는 이걸 보고 팀원 모두가 동일한 작업을 함께 하셨나 하는 생각이 들었거든요.
혹시 이렇게 설정하신 의도가 따로 있으셨다면 함께 이야기해보는 게 좋을 것 같습니다. :)
| from add import add | ||
| from sub import sub | ||
| from mul import mul | ||
| from div import div |
There was a problem hiding this comment.
PR에서 변경된 파일을 확인하다가 흥미로운 부분을 발견했어요.
현재 이 브랜치에는 add.py, sub.py, mul.py, div.py 파일이 존재하지 않는데,
코드에서는 이 파일들을 import하고 있더라고요.
즉, 지금 이 브랜치만 단독으로 실행하면 코드가 정상적으로 동작하지 않아요.
몇 가지 여쭤봐도 될까요?
이 파일들이 없는 상태에서 import를 작성하신 이유가 있으셨나요?
또, 이 코드가 정상적으로 동작하려면 어떤 조건이 갖춰져야 한다고 생각하시나요?
브랜치를 나눠서 작업하는 방식 자체는 좋은 접근이에요.
다만, 이런 상황에서 브랜치 간 의존 관계를
어떻게 관리하면 좋을지에 대해서도 한번 생각해보시면 좋을 것 같습니다.
There was a problem hiding this comment.
말씀 주신 부분 확인했습니다!
현재 main.py에서 add, sub, mul, div를 import한 이유는
해당 연산 모듈들이 팀원분들에 의해 구현될 것을 전제로,
최종적으로 모든 기능이 합쳐졌을 때의 실행 흐름을 기준으로 작성했기 때문입니다.
즉, 저는 이번 작업에서
연산 함수들을 호출하는 메인 로직을 먼저 구성하는 역할이라고 생각하고 구현했습니다.
다만 말씀해주신 것처럼 현재 PR 브랜치만 기준으로 보면
연산 모듈 파일이 포함되어 있지 않아 단독 실행이 불가능한 상태이고,
이 부분은 PR 단위의 독립성과 검증 가능성을 충분히 고려하지 못한 부분이라고 생각합니다.
정상적으로 동작하려면
add.py, sub.py, mul.py, div.py 파일이 동일 경로에 함께 존재해야 합니다.
이번 피드백을 통해
브랜치 간 의존성이 있는 경우에는 PR 설명에 이를 명확히 작성하거나,
가능하면 PR 단독으로도 실행 가능하도록 작업 단위를 나누는 것이 더 적절하다는 점을 배웠습니다.
말씀 주신 부분 반영해서 구조를 다시 점검해보겠습니다.
좋은 피드백 감사합니다!
| num1 = input("첫번째 숫자 입력: ") | ||
| op = input("연산 선택 (+ - * /): ") | ||
| num2 = input("두번째 숫자 입력: ") | ||
|
|
||
| # 입력한 연산자에 따라 알맞은 함수 호출 | ||
| if op == "+": | ||
| result = add(num1, num2) | ||
| elif op == "-": | ||
| result = sub(num1, num2) | ||
| elif op == "*": | ||
| result = mul(num1, num2) | ||
| elif op == "/": | ||
| result = div(num1, num2) | ||
| else: | ||
| result = "잘못된 연산자입니다" |
There was a problem hiding this comment.
PR에서 변경된 코드를 보면, 각 연산 함수를 add(num1, num2) 형태로 호출하고 있어요.
그런데 다른 팀원분들이 구현하신 add.py, sub.py 등의 함수를 보면, 함수 내부에서 직접 input()으로 값을 입력받는 구조로 작성되어 있더라고요. 즉 지금 소영님이 작성하신 방식대로 인자를 넘겨서 호출하면, 실제로는 정상적으로 동작하지 않아요.
이 부분에서 궁금한 게 생겼는데요.
각 파일의 함수가 어떤 형태로 작성되어 있는지, 팀원분들과 미리 맞춰보셨나요?
예를 들어 "함수는 값을 인자로 받을지, 아니면 함수 내부에서 직접 입력을 받을지", "결과를 return할지, print할지" 같은 부분들이요.
브랜치를 나눠서 각자 작업하는 방식은 좋은 접근이에요. 다만 이렇게 나눠서 작업할 때는, 합치기 전에 함수의 입출력 형태를 먼저 합의하는 것이 중요해요. 이걸 인터페이스 정의라고 하는데, 이 부분을 팀 내에서 어떻게 맞추셨는지 궁금합니다 :)
There was a problem hiding this comment.
팀원들이 각 기능을 따로 구현한 뒤, merge 단계에서 파일이 통합되는 것을 전제로 작업을 진행했습니다.
그래서 함수의 입출력 형태를 사전에 명확하게 맞추기보다는, “최종적으로 합쳐졌을 때 동작하도록 구성하면 된다”, "이렇게 코드를 짜면 나중에 합쳐졌을 때 작동이 될 것이다."라는 가정 하에 구현을 진행하게 된 것이 문제가 된 것 같습니다.
"연산 함수들을 “입력을 인자로 받아 결과를 반환하는 형태”로 통일하고,
main.py와 일관되게 동작하도록 하자." 등의 명확한 정의 없이 구두로만 진행되었다는 점에서 팀원간 코드 불일치가 발생한 것 같습니다.
|
@dohun0310 말씀 주신 내용들 전체적으로 확인했습니다!
그래서 이번 PR은 '주요 작업 내용'과 '관련 이슈' 중심으로 간단하게 작성했습니다.
다만 이번에 구현한 이러한 흐름을 담당하는 핵심 역할이라는 의미에서 '메인 기능'이라고 표현했는데, 또한 브랜치 이름에 'main'이 포함되어 Git의 main 브랜치와 혼동될 수 있다는 점도 인지하지 못했는데,
이번에는 작업 단위가 비교적 단순하다 보니 Issue와 PR을 유사하게 작성했는데, 앞으로는
이번에는 코드를 팀원들과 함께 공유하고 같이 확인하면서 진행했다는 의미에서 다만 이렇게 설정할 경우 실제 담당자가 불명확해질 수 있다는 점을 인지했고,
전체적으로 말씀 주신 피드백을 통해 좋은 피드백 감사합니다! |
|
@1028ragon 답변 꼼꼼하게 정말 잘 작성해주셨습니다! 답변을 읽으면 읽을수록, 사람이 적은게 아닌 것 같다는 느낌이 들었어요. 
이 결과가 틀리거나, 정확하지 않을 수 있다는 것을 알기에, 모든 과제의 목표은 소영님의 선택을 하나하나 생각해보고 AI를 사용해서 그 결과물을 사용하신다면, 혹시, 달아주신 답변이 소영님이 스스로 생각하시고 고민하며 작성하신게 맞나요? |
|
@dohun0310 답변 감사합니다. 모든 답변은 제 생각과 고민을 기반으로 작성한 것이 맞습니다. 다만, AI는 오직 말투 정리와 요점 요약으로만 사용하였습니다. 임원진분들이 제시한 과제의 목표에 맞게 제 선택에 대해 스스로 생각하고 고민하며 답변을 적은 점은 분명합니다. 감사합니다! |
6 participants
사용자 입력을 받고, 입력한 연산자에 따라 알맞은 연산 함수를 호출한 뒤
결과를 출력하는 메인 실행 흐름 구현
작업 내용
+,-,*,/)에 따라 함수 분기 처리실행 흐름
add,sub,mul,div호출실행 방법
src폴더로 이동python main.py실행관련 이슈
closes #7