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

전남대 BE_안원모 1주차 과제(2단계) #183

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

Conversation

Wonmoan
Copy link

@Wonmoan Wonmoan commented Jun 28, 2024

코드 리뷰 시, 멘토님이 중점적으로 리뷰해줬으면 하는 부분
전체적으로 아직 Spring의 이해도가 낮아서 제대로 수행했는지 모르겠습니다.. 전반적으로 확인해주시면 감사하겠습니다!
그리고 잘 모르다보니 스스로 구현했다기 보다는 구글링에 의존하여 하였는데,
1.제대로 구현한 것인지
2.코드에 불필요한 점은 없는지
궁금합니다.

Copy link

@pkeugine pkeugine left a comment

Choose a reason for hiding this comment

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

안녕하세요 원모님!
pr 세 개를 걸쳐 리뷰 남겨봤어요.
슥 한 번 봐주시고 반영할 점은 반영하고, 답변은 댓글로 남겨주시면 제가 나중에 확인해볼게요 :)

제대로 구현을 했는지 궁금해하셨는데,
굉장히 잘 만들어주셨습니다. 진심으로요!
불필요한 점은 코멘트로 남겨보았으니 한 번 확인해주세요.
jpa 의존성 외에는 거의 대부분 컨벤션 관련된 불필요한 부분이라
크게 걱정할 요소는 아니라고 생각해요.

주말 잘 보내시고, 같이 파이팅해봐요 우리! 💪 💪 💪

README.md Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved

@GetMapping
public List<Product> getProducts() {
return productService.findAllProducts();

Choose a reason for hiding this comment

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

ResponseEntity 부분이 사라졌네요.
제가 물음표 빌런이라, 이를 왜 없애주셨는지 역시 궁금해요 :)

src/main/java/gift/ProductRepository.java Outdated Show resolved Hide resolved
Comment on lines 11 to 14
@Autowired
public ProductService(ProductRepository productRepository) {
this.productRepository = productRepository;
}

Choose a reason for hiding this comment

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

생성자를 통한 bean 주입을 할 때 보통 @Autowired 를 빼주셨는데,
이부분만 어노테이션을 달아주신 이유가 있나요?

}
return "redirect:/products";
}
@GetMapping("/products/edit/{id}")

Choose a reason for hiding this comment

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

섬세하게 애플리케이션을 아주 잘 만들어주셨네요 👍

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