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

[IDLE-000] 요양보호사는 워크넷 공고를 카드로 볼 수 있고 상세정보를 확인할 수 있다. #56

Merged
merged 17 commits into from
Sep 7, 2024

Conversation

J0onYEong
Copy link
Contributor

변경된점

  • 워크넷 공고 카드 셀 & 상세보기
  • 즐겨찾기 API 변동으로 인한 구현수정

워크넷 공고 카드 셀 & 상세보기

기존의 앱내 공고의 카드및 상세보기의 UI를 재사용하고 싶었지만 생각보다 다른 부분이 많았습니다. 따라서 각자 다른 테이블 뷰 셀과 뷰컨트롤러를 두었습니다.

  • 시현영상

두가지 공고타입(앱내, 워크넷)을 하나의 리스트에서 관리하기 위해 추상화 타입을 사용하였습니다.
즐겨찾기된 공고들의 경우 전혀다른 두 타입이지만 beFavoritedTime값을 기준으로 정렬되어 화면에 표시됩니다.

public protocol RecruitmentPostForWorkerRepresentable {
    var postType: RecruitmentPostType { get }
    var beFavoritedTime: Date? { get }
}

각각의 공고데이터를 가지고있는 VO들은 해당 프로토콜을 채택합니다.

public struct WorknetRecruitmentPostVO: RecruitmentPostForWorkerRepresentable { ... }

public struct NativeRecruitmentPostForWorkerVO: RecruitmentPostForWorkerRepresentable { ... }

@J0onYEong J0onYEong merged commit f1b24e8 into develop Sep 7, 2024
1 check passed
@J0onYEong J0onYEong deleted the feature/workNet_post branch September 7, 2024 03:07
Copy link
Collaborator

@monibu1548 monibu1548 left a comment

Choose a reason for hiding this comment

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

PR 올라오는 것들 중 기능 추가같은 것들은 변경사항 20003000 라인 정도로 꽤 많은 양의 코드가 많은데요.
하나의 PR에 포함된 변경 라인 수를 1000라인 미만으로 잡아보도록 연습해보시는 것을 추천드립니다.
되도록 300
500라인 내외 정도의 변경라인을 갖는 것이 좋습니다.

하나의 PR이 기능의 전체 완성본일 필요는 없습니다.
예를들면 API 코드 추가, UI, API와 UI의 연동. 이렇게 3개로 나눌수도 있구요.

대신,

  • 반드시 빌드가 되는 상태여야 함
  • PR 본문에 어떤 변경사항인지 포함해야 함 (지금 너무 잘하고 계십니다)

리뷰어의 입장에서 200줄 PR 5개 리뷰하는게 1000줄 PR 리뷰하는 것보다 훨씬 빠를겁니다 :)

@J0onYEong
Copy link
Contributor Author

넵 멘토님,, 아직 PR짜르는게 어렵습니다 ㅠㅠ 말씀주신 것처럼 세분화 해보겠습니다!! 코멘트 감사합니다🙇‍♂️🙇‍♂️

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