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

don't include excluded properties #169

Merged

Conversation

fuhrysteve
Copy link
Contributor

Hi @kvesteri! I think this should fix #152 - it seems to for me. Wondering if you could take a look?

@fuhrysteve
Copy link
Contributor Author

In a nutshell, this makes items in __versioned__['exclude'] honor many to many relationships, and not just columns.

@kvesteri
Copy link
Owner

kvesteri commented Sep 7, 2017

Thanks for the PR. Before merging this needs tests

@fpruitt
Copy link
Contributor

fpruitt commented Oct 4, 2017

@kvesteri @fuhrysteve I can confirm that this fixes #152 (an issue I spent several hours debugging today).. I'll look into providing a test to get this ready to merge later on tonight.

@fuhrysteve
Copy link
Contributor Author

@fpruitt that would be awesome! Sorry I haven't circled back to write tests myself

@fuhrysteve fuhrysteve force-pushed the dont_include_excluded_relationships branch from a8efbce to f1d2788 Compare October 11, 2017 14:18
@fuhrysteve
Copy link
Contributor Author

@kvesteri added some tests!

@fpruitt
Copy link
Contributor

fpruitt commented Oct 12, 2017

Until this is merged, my company is installing from this commit (in case there is a delay getting this merged)

fpruitt@4d850b6

@fuhrysteve fuhrysteve force-pushed the dont_include_excluded_relationships branch from 9b19b21 to d540483 Compare November 8, 2017 22:53
@fuhrysteve fuhrysteve force-pushed the dont_include_excluded_relationships branch from fc7d6c0 to a9ac2fb Compare November 8, 2017 23:02
@fuhrysteve
Copy link
Contributor Author

Removed the commit where I tried (and failed) to fix some weirdness with the build failing due to a problem w/ mysql on travisci - it appears to be fixed upstream now.

Additionally, I added python 3.6 (and removed 3.3) from the build and setup.py

@kvesteri - let me know if there's anything else I can do to polish this up - Thanks!

@vincentwhales
Copy link

Please merge this, thank you.

@kvesteri kvesteri merged commit 5e00c0d into kvesteri:master Mar 7, 2018
@kvesteri
Copy link
Owner

kvesteri commented Mar 7, 2018

Thanks for the PR! 🚋

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.

Unable to exclude many-to-many relationships
4 participants