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

SPM number10 (#1487) #1521

Closed
wants to merge 1 commit into from
Closed

Conversation

lorixx
Copy link
Contributor

@lorixx lorixx commented Aug 14, 2021

Summary:

Changes in this pull request

A better version of #1465 =)

  • SPM support with script-based generations.

  • added macOS Catalyst support

Generate SPM layout

  1. From project's root run:

    bash scripts/generate_spm_sources_layout.sh

  2. Commit Changes

Repeat those steps each time you delete/add the project's files. Make sure to have this CI step which will check that generate_spm_sources_layout.sh is not broken.

Issue fixed: #1368 #1406

Checklist

  • All tests pass. Demo project builds and runs.
  • I added tests, an experiment, or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have reviewed the contributing guide

Pull Request resolved: #1487

Reviewed By: candance

Differential Revision: D25562739

Pulled By: lorixx

Summary:
## Changes in this pull request

 A better version of Instagram#1465 =)

- SPM support with script-based generations.

- added macOS Catalyst support

 ### Generate SPM layout

1. From **project's root** run:

   `bash scripts/generate_spm_sources_layout.sh`

  2. Commit Changes

 Repeat those steps each time you delete/add the project's files. **Make sure** to have this CI step which will check that `generate_spm_sources_layout.sh` is not broken.

Issue fixed: Instagram#1368 Instagram#1406

### Checklist

- [ ] All tests pass. Demo project builds and runs.
- [ ] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)

Pull Request resolved: Instagram#1487

Reviewed By: candance

Differential Revision: D25562739

Pulled By: lorixx

fbshipit-source-id: eb4f9e82e6b4842aae71585e0c1377c13cf21196
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25562739

@lorixx
Copy link
Contributor Author

lorixx commented Aug 14, 2021

@3a4oT I rebased to the latest master and tried another time to resolve all the issues for SPM. However, I am still getting some errors from the build. Feel free to piggy back on my rebase and continue the work, I think we are getting really close here! I removed some of the IGListDiff Internal files as I believe they are just a blocker for us.

Test Plan:

  1. Run bash scripts/generate_spm_sources_layout.sh to make sure we generate the symlinks;
  2. Run swift build, <--- validate that we are seeing UIKit/UIKit.h not found errors here;
  3. Run swift package generate-xcodeproj && open ./IGListKit.xcodeproj then in Xcode, hit the Run button, then confirmed there are build errors regarding to the Duplicate symbols.

Screen Shot 2021-08-14 at 11 40 26 AM

Screen Shot 2021-08-14 at 11 40 57 AM

Thanks @3a4oT !

@3a4oT
Copy link
Contributor

3a4oT commented Aug 15, 2021

I'll take a look in the evening or later this week! Thanks!

@lorixx
Copy link
Contributor Author

lorixx commented Aug 16, 2021

Thanks! Feel free to update it in your original PR in #1487 !

@lorixx
Copy link
Contributor Author

lorixx commented Aug 16, 2021

Update:

@3a4oT I shipped the Github Action PR #1478 and working on resolving some of the CI issue, hopefully this saved you some time and focus on the SPM change. Thanks!

(Headsup on the merge conflicts thou!)

@lorixx
Copy link
Contributor Author

lorixx commented Aug 17, 2021

@3a4oT Once you incorporate my change in this PR, I will close this PR out and we can consolidate all in your PR #1487

@lorixx
Copy link
Contributor Author

lorixx commented Aug 18, 2021

Closing this experimental PR #1487

@lorixx lorixx closed this Aug 18, 2021
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.

Xcode 11 SPM Support/Release
3 participants