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

WIP: Do not clone cross references twice #554

Closed
wants to merge 8 commits into from

Conversation

saschahofmann
Copy link

@saschahofmann saschahofmann commented Feb 11, 2022

As explained in #544, I am trying to keep track of instances that have been cloned before and reuse that copy for later references that otherwise would be cloned again.

I probably still need to write a lot more tests but before I do that I wanted to ask whether you're happy with the direction @jackton1 ?

I added on example test, are you ok with me adding cross-references between the tables or should I create new models and do you prefer one big test for all cases or individual ones?

@pep8speaks
Copy link

pep8speaks commented Feb 11, 2022

Hello @saschahofmann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 501:41: E231 missing whitespace after ':'
Line 501:41: E701 multiple statements on one line (colon)
Line 501:42: E225 missing whitespace around operator
Line 623:36: E231 missing whitespace after ':'
Line 623:36: E701 multiple statements on one line (colon)
Line 623:37: E225 missing whitespace around operator

Comment last updated at 2022-03-23 10:20:49 UTC

@saschahofmann
Copy link
Author

@jackton1 do you have any feedback on this? If this is a change you want I'll push it over the finish line!

@jackton1
Copy link
Member

@jackton1 do you have any feedback on this? If this is a change you want I'll push it over the finish line!

Hi @saschahofmann, sorry for the late response, I’ll be happy to review the change when it’s ready for review.

@saschahofmann
Copy link
Author

Closing in favor of #579

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

3 participants