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

Exclude test resource and framework paths from aggregate targets #7000

Merged

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Sep 1, 2017

No description provided.

@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch 3 times, most recently from 253c295 to 7a5bd48 Compare September 1, 2017 20:38
@@ -200,56 +200,59 @@ def supported_test_types
# @return [Array<Hash{Symbol => [String]}>] The vendored and non vendored framework paths
# this target depends upon.
#
def framework_paths
@framework_paths ||= begin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the @framework_paths ||= begin this is why the diff looks weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of removing the caching? This is kinda expensive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So now we are parameterized whether we return paths including the ones from tests or not.

I can actually cache it where the key includes the boolean if you'd like. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cache is now added back and it depends on the parameter.

@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch from 7a5bd48 to 79daf74 Compare September 1, 2017 20:45
:input_path => build_product_path('${BUILT_PRODUCTS_DIR}'),
:output_path => "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/#{product_name}" }
def framework_paths(include_test_framework_paths = true)
accessors = file_accessors.flat_map
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the flat_map here with no block for?

@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch 3 times, most recently from 3269c95 to 710dcfe Compare September 1, 2017 21:06
@dnkoutso dnkoutso changed the title Do not return test resources and frameworks for aggregate targets Exclude test resources and vendored frameworks into aggregate targets Sep 1, 2017
@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch from 710dcfe to 060048f Compare September 5, 2017 19:15
@dnkoutso dnkoutso changed the title Exclude test resources and vendored frameworks into aggregate targets Exclude test resources and vendored frameworks from aggregate targets Sep 6, 2017
@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch from 060048f to 7408faf Compare September 7, 2017 16:01
@dnkoutso dnkoutso changed the title Exclude test resources and vendored frameworks from aggregate targets Exclude test resource and framework paths from aggregate targets Sep 9, 2017
@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch from 7408faf to 4a6dad4 Compare September 9, 2017 21:29
@dnkoutso
Copy link
Contributor Author

dnkoutso commented Sep 9, 2017

Just amended the commit message to be more accurate. No code changes on the last push.

@dnkoutso dnkoutso added this to the 1.4.0 milestone Sep 9, 2017
@dnkoutso dnkoutso force-pushed the test_resources_not_on_aggregates branch from 4a6dad4 to 000ca1f Compare September 11, 2017 18:05
@dnkoutso
Copy link
Contributor Author

All green!

@dnkoutso dnkoutso merged commit c038807 into CocoaPods:master Sep 12, 2017
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.

None yet

3 participants