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

[오정민] Sprint4 #141

Conversation

ojm51
Copy link
Collaborator

@ojm51 ojm51 commented Jun 19, 2024

멘토에게

  • 코드리뷰 사항들을 최대한 반영해보았습니다.
  • 변수명을 재정의하였습니다. ex) 줄여서 쓴 것을 풀어쓰고, 불린값을 담는 변수는 is를 붙여 1/0이 아닌 true/false를 할당,... 등
  • 공통 부분을 가능한 별도의 함수로 선언하고 모듈로 빼냈습니다.
  • 이벤트 핸들러 부분도 모듈화 할 수 있을 거 같은데 해당 작업은 천천히 해보겠습니다🥹
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@ojm51 ojm51 requested a review from Taero-Kim June 19, 2024 12:29
target.setAttribute("data-valid", "true");
};

const activeButton = (button) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

p5;
active -> activate 동사로 바꿔도 좋을 것 같아요!

loginBtn.disabled = true;
loginBtn.classList.remove("enabled");
const checkInputs = () => {
const isValid =
Copy link
Collaborator

Choose a reason for hiding this comment

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

p5;
isValid -> isAllInputValid 도 괜찮을 것 같아요!

@Taero-Kim
Copy link
Collaborator

굳굳!! 1차 버전보다 훨씬 더 가독성이 좋아지고 깔끔해진 것 같아요!
변수명 함수명 네이밍 유념하면서 앞으로의 미션도 지금처럼만 하시면 코드 퀄리티가 쑥쑥 좋아지실 것 같아요!

@Taero-Kim Taero-Kim merged commit 4f30c65 into codeit-bootcamp-frontend:Basic-오정민 Jun 20, 2024
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.

2 participants