Skip to content

Migration to django-popolo part 2#1195

Merged
mhl merged 6 commits intoreviewed-migrate-to-django-popolofrom
migrate-to-django-popolo-part-2
Sep 23, 2016
Merged

Migration to django-popolo part 2#1195
mhl merged 6 commits intoreviewed-migrate-to-django-popolofrom
migrate-to-django-popolo-part-2

Conversation

@mhl
Copy link
Collaborator

@mhl mhl commented Sep 23, 2016

This is the second part of: #1181 for review. See that pull request for the context for this pull request.

This removes the old popit-django model references.

@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage decreased (-69.7%) to 28.789% when pulling 0459ac7 on migrate-to-django-popolo-part-2 into bc4d6c6 on reviewed-migrate-to-django-popolo.

field=models.ForeignKey(to='popolo.Person'),
preserve_default=True,
),
migrations.AlterField(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, this AlterField and the next one are spurious - they were accidentally generated by having the old models file present after 1a0c81a and running makemigrations, I'll add a fixup to remove them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And actually the same spurious AlterFields got into: b6fbffb

@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage decreased (-69.7%) to 28.789% when pulling 03f486d on migrate-to-django-popolo-part-2 into bc4d6c6 on reviewed-migrate-to-django-popolo.

Copy link
Contributor

@chrismytton chrismytton left a comment

Choose a reason for hiding this comment

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

I've made a small suggestion, but assuming the tests will be fixed in part 3, 👍 🚀

contact_type = models.ForeignKey('ContactType')
person = models.ForeignKey(Person)
popolo_person = models.ForeignKey(PopoloPerson, null=True, blank=True)
person = models.ForeignKey(PopoloPerson)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can import PopoloPerson as Person now that we've removed the reference to the popit.models.Person?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I'll do that.

@mhl mhl force-pushed the reviewed-migrate-to-django-popolo branch from bc4d6c6 to 60452ec Compare September 23, 2016 15:25
@mhl mhl force-pushed the migrate-to-django-popolo-part-2 branch from 03f486d to b059c76 Compare September 23, 2016 15:26
@mhl mhl force-pushed the migrate-to-django-popolo-part-2 branch from b059c76 to 235c993 Compare September 23, 2016 15:33
@mhl mhl merged commit 235c993 into reviewed-migrate-to-django-popolo Sep 23, 2016
@mhl mhl removed the in progress label Sep 23, 2016
@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage decreased (-69.7%) to 28.789% when pulling 235c993 on migrate-to-django-popolo-part-2 into 60452ec on reviewed-migrate-to-django-popolo.

@coveralls
Copy link

Coverage Status

Coverage decreased (-69.7%) to 28.789% when pulling 235c993 on migrate-to-django-popolo-part-2 into 60452ec on reviewed-migrate-to-django-popolo.

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