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.

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

Django fields testing #153

Merged
merged 3 commits into from
Jun 7, 2021
Merged

Conversation

abhaykatheria
Copy link
Contributor

This commit adds test for different django fields and
some of their parameters.

This commit also adds model-bakery as a requirement which
is used to create mock objects of model classes and facilitates
testing.

work towards vitessio/vitess#7905

Signed-off-by: abhaykatheria [email protected]

@abhaykatheria abhaykatheria force-pushed the django-fields branch 9 times, most recently from 87a5f79 to 0854d55 Compare June 2, 2021 10:21
This commit adds test for different django fields and
some of their parameters.

This commit also adds model-bakery as a requirement which
is used to create mock objects of model classes and facilitates
testing.

work towards vitessio/vitess#7905

Signed-off-by: Abhay Katheria <[email protected]>
@abhaykatheria abhaykatheria marked this pull request as ready for review June 2, 2021 11:38
Copy link
Contributor

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

LGTM 🐎

Copy link
Contributor

@mcronce mcronce left a comment

Choose a reason for hiding this comment

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

Other than that this looks good to me, thanks!

frameworks/python/django/Dockerfile Outdated Show resolved Hide resolved
… to be persistent and updated the url for django fields guide
Copy link
Contributor

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

All the requested changes have been fixed! 🚤

Copy link
Contributor

@mcronce mcronce left a comment

Choose a reason for hiding this comment

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

@abhaykatheria sorry, I didn't catch the notification that you'd pushed an update yesterday :)

This is fine to merge, I'll push a follow up afterward to explain by example exactly what I'm looking for, and I'll throw some comments in to make it more clear

Thanks!

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