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

Support On-Demand Resource #742

Closed
wants to merge 3 commits into from
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions lib/xcodeproj/project/object/native_target.rb
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,36 @@ def add_resources(resource_file_references)
end
end

# remove on demand resource files to the resources build phase of the target.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit capital case 'R'

#
# @param [Array<PBXFileReference>] on_demand_resource_file_references
# the files references of the on demand resources to the target.
#
# @return [void]
#
def remove_on_demand_resources(on_demand_resource_file_references)
on_demand_resource_file_references.each do |file|
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is already defined? on_demand_resource_file_references?

Copy link
Contributor

Choose a reason for hiding this comment

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

nm I am stupid and this is a parameter. Hmmmm this doesn't really match what the function does. Its just removing resource file reference which can be done in remove_file_reference inside build_phase.rb. I don't think this method is really needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or at least can we assert that ASSET_TAGS is present on these resources?

Copy link
Author

Choose a reason for hiding this comment

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

emmm... Where do I write it more reasonable?

resources_build_phase.remove_file_reference(file)
end
end

# Adds on demand resource files to the resources build phase of the target.
#
# @param [Array<PBXFileReference>] on_demand_resource_file_references
# the files references of the on demand resources to the target.
#
# @return [void]
#
def add_on_demand_resources(on_demand_resource_file_references)
on_demand_resource_file_references.each do |file|
next if resources_build_phase.include?(file)
build_file = project.new(PBXBuildFile)
build_file.file_ref = file
build_file.settings = (build_file.settings ||= {}).merge({"ASSET_TAGS" => ["OnDemand"]})
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to be hard-coding asset tags here. According to Apple's docs tags are supposed to be manually specified, and those tags are then used in code to download the assets

Copy link
Author

Choose a reason for hiding this comment

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

thanks, now use the podspec on_demand_resources specified tag
81403b6#diff-5ee67a82fd5ae8aa882ddaf26476e138R577

resources_build_phase.files << build_file
end
end

# Finds or creates the headers build phase of the target.
#
# @note A target should have only one headers build phase.
Expand Down