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

New mapping macros using VOKKeyPathHelper #73

Merged
merged 22 commits into from
Feb 8, 2016

Conversation

chillpop
Copy link
Contributor

@chillpop chillpop commented Feb 7, 2016

@vokal-isaac started with this:

  • New mapping macros using the VOKUtilities/VOKKeyPathHelper for better type checking.
  • Deprecate the VOK_CDSELECTOR and VOK_MAP_FOREIGN_TO_LOCAL macros.

I took over and did this:

  • Muck around with module maps to allow using a "non-modular header inside of a framework module"
  • Make a new subspec for the mapping macros
  • Update Travis to use Xcode 7.2

@vokal/ios-developers, @seanwolter, code review?

vokal-isaac and others added 20 commits February 3, 2016 11:39
Deprecated VOK_MAP_FOREIGN_TO_LOCAL in favor of two new helper macros, VOK_CDSELECTOR in favor of macros in VOKKeyPathHelper.
specify the class name before the class property name to let Xcode help with autocomplete.
Use a module map to include the non-module header VOKKeyPathHelper.
This fixes the "include of non-modular header inside framework module" error in Xcode 7 when use_frameworks is enabled.

This subspec is included by default for Obj-C, but excluded for Swift because the macros are only usable from Obj-C code.
@seanwolter
Copy link
Contributor

Grate werk separating pod updates from the rest of the change 😑

@@ -50,7 +52,7 @@ or

Vokoder offers a lightweight mapper for importing Foundation objects into Core Data. Arrays of dictionaries can be imported with ease once maps are set up. If no maps are provided Vokoder will use its default maps. The default maps assume that foreign keys have the same names as your core data attributes. It will make its best effort to identify dates and numbers.

Setting up your own maps is recommended. Macros are provided to make it fun and easy. Below is an example of setting up a mapper for an arbitrary managed object subclass. Mappers are not persisted between app launches, so be sure to setup your maps every time your application starts.
Setting up your own maps is recommended. Macros are provided to make it fun and easy. Below is an example of setting up a mapper for a managed object subclass `VOKPerson`. Mappers are not persisted between app launches, so be sure to setup your maps every time your application starts.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Macros are provided to make it fun and easy" 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you may have written that :suspect:

Copy link
Contributor

Choose a reason for hiding this comment

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

I am sooooo charming!

@chillpop
Copy link
Contributor Author

chillpop commented Feb 8, 2016

I can redo this with the pod updates split out, but Travis will fail.

@seanwolter
Copy link
Contributor

I wasn't seriously complaining about the pods.
LGTM

*/
#ifndef VOKMapForeignToLocalClassProperty
# define VOKMapForeignToLocalClassProperty(inputKeyPath, klass, coreDataSelectorSymbol) \
[VOKManagedObjectMap mapWithForeignKeyPath:inputKeyPath coreDataKey:VOKKeyForInstanceOf(klass, coreDataSelectorSymbol)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Were it me, I'd indent these macro line continuations to be one level deeper than the define, but Xcode will keep trying to put them back outdented to here, so it may not be worth it.

@vokal-isaac
Copy link
Contributor

Given the project file noise, we probably should apply xUnique to all the project files, but I don't know that it's worth doing on this PR.

@chillpop
Copy link
Contributor Author

chillpop commented Feb 8, 2016

README formatting updates pushed

@@ -170,7 +172,7 @@ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_LOW, 0), ^{
NSManagedObjectContext *backgroundContext = [[VOKCoreDataManager sharedInstance] temporaryContext];

SomeManagedObjectSubclass *thing = [SomeManagedObjectSubclass vok_newInstanceWithContext:backgroundContext];
thing.someArbitrayAttribute = @"hello";
thing.someArbitrayAttribute = @"hello";
[[VOKCoreDataManager sharedInstance] saveAndMergeWithMainContext:backgroundContext];
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation here looks funny to me. Am I missing something? Is it a GitHub quirk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed converting a tab to spaces

@vokal-isaac
Copy link
Contributor

image

@chillpop
Copy link
Contributor Author

chillpop commented Feb 8, 2016

That goat does not want to dance

@chillpop
Copy link
Contributor Author

chillpop commented Feb 8, 2016

I’ll make another PR adding xUnique before releasing these macro changes to pod trunk

chillpop added a commit that referenced this pull request Feb 8, 2016
New mapping macros using VOKKeyPathHelper
@chillpop chillpop merged commit 7e20128 into vokal:master Feb 8, 2016
@chillpop chillpop deleted the feature/VOKKeyPathHelper branch February 8, 2016 17:15
@seanwolter
Copy link
Contributor

Wow. @vokal-isaac Great "goat to pull"

@designatednerd
Copy link
Contributor

@seanwolter @vokal-isaac For real. Saved for future use.

@vokal-isaac
Copy link
Contributor

Every once in a while, particularly for @chillpop, I try to find one we haven't used before.

@chillpop chillpop mentioned this pull request Feb 10, 2016
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.

4 participants