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.

Sign up for GitHub

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

Jump to bottom

Factories cleanup #808

Merged
merged 1 commit into from Feb 27, 2017
Merged

Factories cleanup #808

merged 1 commit into from Feb 27, 2017

Conversation

noirbizarre
Copy link
Contributor

This PR cleanup all factories

  • make use of factory.Faker
  • remove duplicate factories
  • use the same factory pattern everywhere (no more global udata.model import, use factory import as much as possible)
  • put all testing helpers as Faker providers

@noirbizarre noirbizarre added this to the 1.0.4 milestone Feb 27, 2017
@noirbizarre noirbizarre mentioned this pull request Feb 27, 2017
Closed
davidbgk
davidbgk suggested changes Feb 27, 2017
View reviewed changes
Copy link
Member

@davidbgk davidbgk left a comment

Choose a reason for hiding this comment

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

Thanks a lot for splitting, it really helps! 👍

udata/utils.py

Might be conributed to upstream Faker project
'''
def unique_string(self, length=UUID_LENGTH):
Copy link
Member

Choose a reason for hiding this comment

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

why not unique_string = unique_string ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it works with method = function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed: I just tested and method = function does not work 😞

Copy link
Member

Choose a reason for hiding this comment

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

mmh I see, you'll have to deal with self with a partial and add extra complexity. Let's keep it as is.

@noirbizarre noirbizarre merged commit fa46c00 into opendatateam:master Feb 27, 2017
@noirbizarre noirbizarre deleted the factories-cleanup branch February 27, 2017 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbgk davidbgk davidbgk approved these changes

@vinyll vinyll Awaiting requested review from vinyll

@jdesboeufs jdesboeufs Awaiting requested review from jdesboeufs

@teleboas teleboas Awaiting requested review from teleboas

Assignees
No one assigned
Labels
refactoring
Projects
None yet
Milestone
1.0.4
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@noirbizarre @davidbgk