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

[#505] Set up necessary library for a SwiftUI project #506

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

phongvhd93
Copy link
Contributor

What happened 👀

Update the profile of SwiftUI project by adding a new library or removing redundant 3rd-parties. For instance, we will add FlowStacks library, which handles the navigation flow in a SwiftUI application.

Insight 📝

Added the following libs:

  • FlowStacks
  • Moya/Combine
  • JSONAPIMapper'

Proof Of Work 📹

This PR must pass the CI

@phongvhd93 phongvhd93 self-assigned this Jul 21, 2023
@phongvhd93 phongvhd93 force-pushed the feature/505-update-podfile branch from f2c0200 to f902844 Compare July 21, 2023 09:33
@phongvhd93 phongvhd93 marked this pull request as draft July 21, 2023 09:36
@phongvhd93 phongvhd93 force-pushed the feature/505-update-podfile branch 2 times, most recently from de28d78 to d0751d4 Compare July 21, 2023 09:55
@phongvhd93 phongvhd93 marked this pull request as ready for review July 21, 2023 10:29

# Backend
pod 'Alamofire'
# Network
Copy link
Member

Choose a reason for hiding this comment

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

Please use either Backend or Network

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved in 232caf4

pod 'Alamofire'
# Network
pod 'Moya/Combine'
pod 'JSONAPIMapper', :git => 'https://github.com/nimblehq/JSONMapper', :branch => 'main'
Copy link
Member

Choose a reason for hiding this comment

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

We have tag, so please use tag instead of branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved in 232caf4

@Thieurom
Copy link

Thieurom commented Jul 24, 2023

@phongvhd93 Is it possible to dynamically change the content of the Podfile depending on whether the project is UIKit or SwiftUI? Besides common libraries, I've seen that some are meant for UIKit or SwiftUI, but not both. For example, if a project is using SwiftUI+Combine, the following libraries should be removed:

pod 'SnapKit'
pod 'RxAlamofire'
pod 'RxCocoa'
pod 'RxDataSources'
pod 'RxSwift'
pod 'RxNimble', subspecs: ['RxBlocking', 'RxTest']

Another simple solution is we'll default the Podfile to, for example, SwiftUI. So all UIKit-related libraries could be commented with an instruction:

# Uncomment these if you're using UIKit:
# pod 'SnapKit'
# pod 'RxSwift'
...

@phongvhd93
Copy link
Contributor Author

@phongvhd93 Is it possible to dynamically change the content of the Podfile depending on whether the project is UIKit or SwiftUI? Besides common libraries, I've seen that some are meant for UIKit or SwiftUI, but not both. For example, if a project is using SwiftUI+Combine, the following libraries should be removed:

pod 'SnapKit'
pod 'RxAlamofire'
pod 'RxCocoa'
pod 'RxDataSources'
pod 'RxSwift'
pod 'RxNimble', subspecs: ['RxBlocking', 'RxTest']

Another simple solution is we'll default the Podfile to, for example, SwiftUI. So all UIKit-related libraries could be commented with an instruction:

# Uncomment these if you're using UIKit:
# pod 'SnapKit'
# pod 'RxSwift'
...

We have 2 separate folders for SwiftUI and UIKit, and also 2 separate podfiles. With the current implementation, it is easier to change the content of the podfile. make.sh command will base on the UI type selection and copy the corresponding files in either UIKit or SwiftUI folder

@Thieurom
Copy link

We have 2 separate folders for SwiftUI and UIKit, and also 2 separate podfiles.

Oh, I missed this part 🙏
It was that the default branch is main and hasn't incorporated the latest changes from develop, so I couldn't find that.
It's all good then 👍

@phongvhd93 phongvhd93 requested review from blyscuit and suho July 31, 2023 03:28
@@ -14,7 +14,8 @@ target '{PROJECT_NAME}' do
pod 'Kingfisher'

# Backend
pod 'Alamofire'
pod 'Moya/Combine'
Copy link
Member

Choose a reason for hiding this comment

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

We need to close this issue #324 first, before adding Moya to other template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

solved in 6346fc6

@blyscuit blyscuit added this pull request to the merge queue Jul 31, 2023
@blyscuit blyscuit removed this pull request from the merge queue due to a manual request Jul 31, 2023
@phongvhd93 phongvhd93 force-pushed the feature/505-update-podfile branch from 232caf4 to 6346fc6 Compare August 2, 2023 02:11
@phongvhd93 phongvhd93 requested a review from suho August 2, 2023 02:12
@blyscuit blyscuit added this pull request to the merge queue Aug 4, 2023
Merged via the queue into develop with commit 1385fce Aug 4, 2023
@blyscuit blyscuit deleted the feature/505-update-podfile branch August 4, 2023 02:49
@blyscuit blyscuit added this to the 4.7.0 milestone Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants