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

Adds handling of recursive mappings #54

Merged

Conversation

josepmedrano
Copy link
Contributor

As discussed on issue 53 (#53)

@dimazen
Copy link
Contributor

dimazen commented Mar 29, 2016

@josepmedrano thanks! I'll take a closer look tomorrow.

@Antondomashnev
Copy link

@dimazen hi, what's the status of this pull request review?

@Antondomashnev
Copy link

Antondomashnev commented Aug 23, 2016

@josepmedrano please add nullability type specifier for all parameters of - (void)addRecursiveRelationshipMappingForProperty:(NSString *)property keypath:(NSString *)keyPath;

I've opened a respective PR in your fork. Unfortunately the tests are still failed, but I don't have time now to fix it.

@dimazen ping =)

@dimazen
Copy link
Contributor

dimazen commented Aug 23, 2016

Hello, @Antondomashnev. Sorry for such a huge delay - I was busy on the project and then totally forgot about FEM and pending PRs.

Going to take a look at your PR today and merge if it is OK (it should be, I reviewed it before).
Also, I can roll out release today (1.1, lets say).

Regarding failing tests: they should be fine - I've fixed Travis CI integration, however, it won't rerun existing PRs unless you'll try to push something :)

Therefore I can merge @josepmedrano's changes and then your request and then merge to the master.

@Antondomashnev
Copy link

@dimazen that's sounds awesome

@dimazen
Copy link
Contributor

dimazen commented Aug 23, 2016

@Antondomashnev I have a weekend tomorrow, so decided to postpone it until tomorrow's morning (since whole day is free). Hope it is ok. (But I'm keeping FEM in mind)

@Antondomashnev
Copy link

@dimazen don't worry I'm using the pod from my fork now, it's not urgent

@dimazen dimazen changed the base branch from master to feature/FEM-53 August 24, 2016 09:36
@dimazen dimazen merged commit dcf67d8 into Yalantis:feature/FEM-53 Aug 24, 2016
@dimazen
Copy link
Contributor

dimazen commented Aug 24, 2016

This PR has failing tests (fixing), I've merged it into current feature/FEM-53 branch where I'm also going to merge @Antondomashnev's changes.

@Antondomashnev
Copy link

Thanks @dimazen 🚀

@dimazen
Copy link
Contributor

dimazen commented Aug 25, 2016

I'm planning to make a release on this weekends, that will also include documentation in headers, so thats why you don't see any new release with recursive mapping

@dimazen
Copy link
Contributor

dimazen commented Aug 29, 2016

@Antondomashnev you can now grab 1.1 release from the CocoaPods :)

@Antondomashnev
Copy link

@dimazen amazing, thanks lot =)

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.

3 participants