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

feat: Create meal widget #116

Merged
merged 8 commits into from
Aug 13, 2024
Merged

feat: Create meal widget #116

merged 8 commits into from
Aug 13, 2024

Conversation

hhhello0507
Copy link
Contributor

No description provided.

.project(target: "Repository", path: .relativeToRoot("Projects/Data")),
.project(target: "DIContainer", path: .relativeToRoot("Projects/DIContainer")),
.project(target: "Shared", path: .relativeToRoot("Projects/Shared")),
.external(name: "DDS")
]
)
]

Choose a reason for hiding this comment

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

코드 패치에 대한 간단한 리뷰입니다:

  1. 의존성 중복: RepositoryDIContainer가 기존 의존성과 새로운 타겟 모두에 추가되었습니다. 각 타겟에서 필요 유무를 검토하여 중복을 제거하는 것이 좋습니다.

  2. 배포 대상: iOS 15.0에 배포하도록 설정한 것은 좋은 접근이지만, 사용자 기반에 따라 이전 버전과의 호환성을 고려할 필요가 있습니다.

  3. 스크립트 지정: .swiftLint 스크립트를 추가하였는데, 이를 통해 코드 일관성을 유지할 수 있습니다. 그러나 해당 스크립트 실행이 실패할 경우 빌드 오류가 발생할 수 있으므로, 적절한 환경설정을 필요합니다.

  4. 잘못된 정보 플리스트 키: infoPlist"NSExtensionPointIdentifier"형식은 맞으나 값이 올바른지 확인하고, 추가적인 필수가 필요한지 검토해야 합니다.

  5. 리소스 및 소스 경로: "iOS-Widget/Source/**""iOS-Widget/Resource/**" 경로가 존재하는지 확인하고, 확장자로 필터링이 제대로 이루어지는지도 체크해야 합니다.

제안:

  • 코드 주석을 추가하여 의존성 또는 설정 이유를 설명하면 가독성을 높일 수 있습니다.
  • 테스트 케이스 및 문서화를 강화하여 새롭게 추가된 위젯 관련 기능이 원활히 작동하는지 확인하는 절차를 마련하시기 바랍니다.

Copy link
Member

@Mercen-Lee Mercen-Lee left a comment

Choose a reason for hiding this comment

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

LGTM 👍 👍

@hhhello0507 hhhello0507 merged commit 145a999 into develop Aug 13, 2024
@hhhello0507 hhhello0507 deleted the feature/widget branch September 10, 2024 08:09
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