Skip to content

Conversation

@joncotton
Copy link
Member

Stop using select_related() on Well.items because we aren't concerned with subclasses of Nodes. Add a test for GenericForeignKeyQuerySet.all(). Removing unused imports. A touch of PEP8.

@dmclain
Copy link
Member

dmclain commented Feb 24, 2012

I'm a bit concerned by the line "Stop using select_related() on Well.items because we aren't concerned with subclasses of Nodes". select_related fetches related objects, not subclasses.

@sergiosoria
Copy link
Contributor

I think we're confusing select_related() with select_subclasses() , in this case.

…king for subclasses of Well Nodes."

This reverts commit db9a961.
@joncotton
Copy link
Member Author

@sergiosoria yes, grrr, thank you. I mistook that method for django-model-utils' InheritanceManager select_subclasses(). Commits reverted.

@tswicegood
Copy link
Contributor

Closing. This doesn't actually change anything now.

As a future note, please do not use the \ syntax to put imports across multiple lines. Multi-line imports should be handled via:

from foo import (a, b, c, etc...
        x, y, z)

Though the preferred method when it's unclear whether the import will remain there is to either import the module or import them on individual lines. Individual lines makes it easier to see what was added/removed later.

@tswicegood tswicegood closed this Feb 27, 2012
@joncotton
Copy link
Member Author

This PR still adds a test for all() on the querysets.

I can change the "" multi-line imports to using parentheses. If you don't care about that, then I can just make a new PR for the additional test.

@tswicegood
Copy link
Contributor

Ahh -- I missed that. Let's open up a new PR with just those changes in them.

@joncotton joncotton mentioned this pull request Mar 9, 2012
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