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

Fixed #30393 -- Added validation of startapp's directory option. #11270

Merged
merged 1 commit into from
Apr 25, 2019

Conversation

MyungSeKyo
Copy link

@felixxm felixxm self-assigned this Apr 24, 2019
Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@MyungSeKyo Thanks for this patch 👍 I left some comments.

django/core/management/templates.py Outdated Show resolved Hide resolved
tests/admin_scripts/tests.py Outdated Show resolved Hide resolved
@MyungSeKyo
Copy link
Author

@felixxm Thanks to your kind review!
I tried to fix according to your review
could you review again?

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@MyungSeKyo Thanks for the update 👍 I left some small comments.

tests/admin_scripts/tests.py Outdated Show resolved Hide resolved
tests/admin_scripts/tests.py Outdated Show resolved Hide resolved
tests/admin_scripts/tests.py Outdated Show resolved Hide resolved
@MyungSeKyo
Copy link
Author

MyungSeKyo commented Apr 25, 2019

@jdufresne @felixxm
Thanks to your reviews 👍
I made test_invalid_target_name and used unittest.subTest
and I squashed commits

@felixxm
Copy link
Member

felixxm commented Apr 25, 2019

@MyungSeKyo Thanks! I rebased from master and pushed small final edits.

@felixxm felixxm changed the title Fixed #30393 -- Added validation to target of startapp command Fixed #30393 -- Added validation of startapp's directory option. Apr 25, 2019
@felixxm felixxm merged commit fc9566d into django:master Apr 25, 2019
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.

4 participants