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

Extend on_demand_resources DSL to allow the ability to specify a category. #697

Merged
merged 1 commit into from Aug 10, 2021

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Aug 9, 2021

No description provided.

@dnkoutso dnkoutso added this to the 1.11.0 milestone Aug 9, 2021
@dnkoutso dnkoutso requested a review from amorde August 9, 2021 22:24
@@ -463,25 +463,37 @@ module Pod

it 'returns the on demand resources specified as a string wrapped in an array' do
@spec.on_demand_resources = { 'Maps' => 'MapView/Map/Resources/*.png' }
@consumer.on_demand_resources.should == { 'Maps' => ['MapView/Map/Resources/*.png'] }
@consumer.on_demand_resources.should == { 'Maps' => { :paths => ['MapView/Map/Resources/*.png'], :category => :on_demand } }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

consumer now normalizes all values to be a hash with 2 keys, :paths and :category in which :paths is always an array and :category a symbol.

result = Specification.from_json(json)
result.on_demand_resources.count.should.equal 3
result.on_demand_resources.should == {
'tag1' => { 'paths' => 'Tag1Resources/*', 'category' => 'on_demand' },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the specification just stores the attribute which means the key can be a string if its coming from JSON. Using consumer would normalize this and ensure all keys are symbols, so this is fine.

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 difference between on_demand and download_on_demand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch this was a bad test! I had started this PR calling it on_demand and then switched to download_on_demand to be explicit. will update!

@dnkoutso dnkoutso changed the base branch from master to 1-11-stable August 9, 2021 22:28
@dnkoutso dnkoutso force-pushed the odr_category branch 2 times, most recently from b4decc5 to 95c9d66 Compare August 9, 2021 22:34
@dnkoutso dnkoutso requested a review from paulb777 August 9, 2021 22:39
@@ -463,25 +463,37 @@ module Pod

it 'returns the on demand resources specified as a string wrapped in an array' do
@spec.on_demand_resources = { 'Maps' => 'MapView/Map/Resources/*.png' }
@consumer.on_demand_resources.should == { 'Maps' => ['MapView/Map/Resources/*.png'] }
Copy link
Member

Choose a reason for hiding this comment

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

Should the deleted line also be supported with a default category?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it actually is. consumer now will always normalize and return a hash with :path and :category. I think the test ensures that if the user doesnt use the hash DSL then consumer would do the right thing.

result = Specification.from_json(json)
result.on_demand_resources.count.should.equal 3
result.on_demand_resources.should == {
'tag1' => { 'paths' => 'Tag1Resources/*', 'category' => 'on_demand' },
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 difference between on_demand and download_on_demand?

@dnkoutso dnkoutso force-pushed the odr_category branch 3 times, most recently from 4ac3f74 to 378c81d Compare August 10, 2021 10:45
# Performs validations related to the `on_demand_resources` attribute.
#
def _validate_on_demand_resources(h)
category = h.dig(h.keys.first, :category)&.to_sym
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is actually wrong, we need to collect all :category from all entries. will fix and update the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed and test updated. pushed.

@dnkoutso
Copy link
Contributor Author

Will be merging this once I get the CocoaPods gem PR ready in case I need something else from it.

@dnkoutso dnkoutso merged commit b3b2436 into CocoaPods:1-11-stable Aug 10, 2021
@dnkoutso dnkoutso deleted the odr_category branch August 10, 2021 21:27
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

2 participants