Skip to content

Conversation

todd-dembrey
Copy link
Contributor

@incuna/backend

@Minglee01 Get your own back time

Copy link
Contributor

@meshy meshy left a comment

Choose a reason for hiding this comment

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

LGTM barring nitpick

fields = get_all_field_names(self.model)

expected = wipe_id_fields_on_django_lt_17((
expected = (
Copy link
Contributor

Choose a reason for hiding this comment

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

As Django 1.7 is no longer supported, it might be worth completely removing the wipe_id_fields_on_django_lt_17 helper method.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is part of incuna_test_utils. @kevinetienne removed it in incuna/incuna-test-utils@8d65296

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, my bad! Getting my wires crossed there! Well investigated, thanks! :)

@adam-thomas
Copy link
Contributor

Nice one! Looks good, HWTI :)

@coveralls
Copy link

coveralls commented Oct 4, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 9af4804 on add_django110_to_the_tests into 7d9a75a on master.

@kevinetienne kevinetienne merged commit ad13e1d into master Oct 4, 2016
@kevinetienne kevinetienne deleted the add_django110_to_the_tests branch October 4, 2016 15:08
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.

5 participants