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.

Sign up for GitHub

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

Jump to bottom

Step2 - GitHub(HTTP) #109

Open
wants to merge 11 commits into
base: kldaji
Choose a base branch
from
Open

Step2 - GitHub(HTTP) #109

wants to merge 11 commits into from

Conversation

kldaji
Copy link

@kldaji kldaji commented Sep 4, 2023

마지막 리뷰가 되겠네요 ㅎㅎ

마지막까지 잘 부탁드립니다!!

AnKyungMoo
AnKyungMoo requested changes Sep 4, 2023
View reviewed changes
Copy link

@AnKyungMoo AnKyungMoo left a comment

Choose a reason for hiding this comment

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

마무리 단계의 미션이어서 그런지 깔끔하게 해결해주고 계시네요!
요구사항중 놓치신 부분이 있어서 코멘트 남겨두었습니다.
수정 후 다시 요청주세요~!

data/src/main/java/camp/nextstep/edu/github/GithubService.kt

import retrofit2.http.GET

interface GithubService {
Copy link

Choose a reason for hiding this comment

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

data 모듈의 구현체는 모두 internal class로 선언한다.
라는 요구사항이 있습니다.
Service 뿐만 아니라 전체적으로 같이 신경써주시면 감사합니다 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnKyungMoo AnKyungMoo AnKyungMoo requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kldaji @AnKyungMoo