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

fix: Remove default support for MacCatalyst in Framework projects #4134

Merged
merged 4 commits into from
Feb 10, 2022
Merged

fix: Remove default support for MacCatalyst in Framework projects #4134

merged 4 commits into from
Feb 10, 2022

Conversation

TheInkedEngineer
Copy link
Contributor

@TheInkedEngineer TheInkedEngineer commented Feb 10, 2022

Resolves #4133
Request for comments document (if applies):

Short description 📝

After a short discussion on slack it was agreed then when the deployment target is explicitly specified to iOS and/or iPad, framework projects should not support the default behaviour of Xcode of including Mac Catalyst by default.

How to test the changes locally 🧐

A unit test has been added to make sure the modifications are properly working.

Checklist ✅

  • The code architecture and patterns are consistent with the rest of the codebase.
  • The changes have been tested following the documented guidelines.
  • The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
  • In case the PR introduces changes that affect users, the documentation has been updated.
  • In case the PR introduces changes that affect how the cache artifact is generated starting from the TuistGraph.Target, the Constants.cacheVersion has been updated.

@danieleformichelli danieleformichelli changed the base branch from main to cocoapods-models February 10, 2022 10:03
@danieleformichelli danieleformichelli changed the base branch from cocoapods-models to main February 10, 2022 10:03
@danieleformichelli
Copy link
Collaborator

@allcontributors add @TheInkedEngineer as contributor for code

@allcontributors
Copy link
Contributor

@danyf90

I've put up a pull request to add @TheInkedEngineer! 🎉

@@ -8,6 +8,8 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/

- Use GitHub tags (via `git ls-remote`) to determine the latest Tuist version when installing/updating Tuist [#3985](https://github.com/tuist/tuist/pull/3985) by [@ezraberch](https://github.com/ezraberch)

- Remove default MacCatalyst support when framework deployment target is set to iOS and/or iPad [#4134](https://github.com/tuist/tuist/pull/4134) by [@TheInkedEngineer](https://github.com/TheInkedEngineer)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why I see this old changelog here 🤔
For example, if you see on main, there should be many more entries in Next, and last release should be 2.7.2 🤷‍♂️

Copy link
Contributor Author

@TheInkedEngineer TheInkedEngineer Feb 10, 2022

Choose a reason for hiding this comment

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

I branched from main, should've I have branched from somewhere else @danyf90 ?
I can rebase

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, branch is fine 🤷

@danieleformichelli
Copy link
Collaborator

To fix linting errors: ./fourier lint tuist --fix
Also, if you want you can setup git hooks running ./fourier up

Copy link
Collaborator

@luispadron luispadron left a comment

Choose a reason for hiding this comment

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

Is this just a confusing thing for users or does it not work? For example, how could someone enable this for both iOS and Catalyst?

Read the slack thread!

@luispadron luispadron merged commit f1a965b into tuist:main Feb 10, 2022
@EthanLipnik
Copy link

Is this just a confusing thing for users or does it not work? For example, how could someone enable this for both iOS and Catalyst?

Read the slack thread!

I can't access the Slack thread without a custom email. How can I enable Catalyst support in my dependencies?

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.

Framework target not adhering to deploymentTarget
5 participants