Skip to content

Conversation

Salakar
Copy link
Member

@Salakar Salakar commented Feb 16, 2020

Summary:

Abstract targets don't support script phases, this change skips adding script phases if the current target is abstract.

Fixes invertase/react-native-firebase#2800

cc @orta / @alloy for review please, thanks!

Test Plan:

@gabrielbull would you be able to patch package this change in and confirm it now works? invertase/react-native-firebase#2800


script phases continue to work on non abstract targets:
image

Copy link
Member

@grabbou grabbou left a comment

Choose a reason for hiding this comment

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

Looks good to me. Going to wait for one more review before moving forward. Thanks for sending over the PR.

Copy link
Member

@alloy alloy left a comment

Choose a reason for hiding this comment

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

Yup, this looks good to me 👍

@thymikee thymikee closed this Feb 17, 2020
@thymikee thymikee reopened this Feb 17, 2020
@thymikee thymikee merged commit 167c474 into master Feb 17, 2020
@thymikee thymikee deleted the @salakar/scripts-abstract-target branch February 17, 2020 11:02
thymikee pushed a commit that referenced this pull request Feb 17, 2020
* fix(ios): skip script phases on abstract targets

Fixes invertase/react-native-firebase#2800

* fix typo

* update tests
@thymikee
Copy link
Member

Thanks @alloy, @Salakar! This is released in v3.2.

@r0ssIV
Copy link

r0ssIV commented Jan 17, 2023

@Salakar @thymikee hello, what about real targets inside abstract targets ? Pls also check link above.

abstract_target 'SomeAbstractTarget' do
  config = use_native_modules!

 .... abbreviated for example....

  target 'RealTarget' do
    # Production-specific pods

    target 'RealTargetTests' do
      inherit! :complete
      # Pods for testing
    end
  end

end



@AuroPick

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autolink bug Something isn't working platform: ios

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants