Skip to content

Conversation

@erictheise
Copy link
Contributor

The crux of this pull request is the deletion of lines 107-111 from rest_framework_gis/serializers.py.

I've also moved all test_*_filtering tests into tests/django_restframework_gis_tests/test_filters.py as a first step in refactoring the unwieldy tests/django_restframework_gis_tests/tests.py. I'm also testing against the whole GeoJSON hash in test_geojson_format instead of simply poking at a few items. This test would have immediately revealed the bug reported in #38.

Finally, changed the POINT coordinates in test_geojson_format so that they can be distinguished from each other, and so that it's a little more intuitive to see which is longitude and which is latitude.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 94.1% when pulling 67fcc55 on erictheise:organize_tests into 4b890a0 on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 94.37% when pulling 67fcc55 on erictheise:organize_tests into 4b890a0 on djangonauts:master.

@erictheise
Copy link
Contributor Author

Any feedback, @nemesisdesign?

@nemesifier
Copy link
Member

It looks good!

it would have been better if the first commit focused on just the issue in subject and if you did the split of the tests (which is a very good idea) in a separate commit.

Do you still have some time to do that or not? Let me know and I'll wait or merge accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

very minor indentation issue here

@erictheise
Copy link
Contributor Author

@nemesisdesign will get to this as a new sequence of commits, thanks.

@erictheise
Copy link
Contributor Author

@nemesisdesign see #42 for a cleaner pull request.

@erictheise erictheise closed this Feb 18, 2015
@nemesifier
Copy link
Member

are you familiar with git rebase -i (interactive)?

@erictheise erictheise deleted the organize_tests branch February 18, 2015 20:49
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