Fixed #14087 -- find management commands in namespace packages #178

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
@bhuztez

bhuztez commented Jun 28, 2012

improve joh's and nfg's patches, find management commands in namespace packages only. packages imported by pep-302 importers should be in another patch.

https://code.djangoproject.com/ticket/14087

orblivion and others added some commits Dec 2, 2012

Merge pull request #569 from orblivion/master
Fixed typo in ValuesQuerySet._as_sql docstring
Fixed #19416 -- Fixed multi-line commands in initial SQL files
Thanks Aymeric Augustin for detecting this regression.
Fixed #19318 -- Ensured that the admin's SimpleListFilter options can…
… be displayed as selected even if the lookup's first element is not a string.
Fixed #18697 -- Made values accepted for two customizable admin templ…
…ates consistent.

Thanks and at cloverfastfood dot com for the report.
Merge pull request #573 from tominsam/master
Fixed #19070: urlize template filter raises exception in some cases
Fixed #19391 -- Oracle specific failure in tests
The failure was caused by using None as a choice for a CharField. To
avoid Oracle's "" <-> NULL handling the field type was changed to
IntegerField.
Amended explanation of LOCALE_PATHS setting.
Thanks Daniele Procida for the patch.
@manfre

This comment has been minimized.

Show comment
Hide comment
@manfre

manfre Dec 10, 2012

Contributor

Why was this test converted to an identity insert? This bypasses SQLCompilers and any backend that needs to do special handling for identity inserts.

Why was this test converted to an identity insert? This bypasses SQLCompilers and any backend that needs to do special handling for identity inserts.

This comment has been minimized.

Show comment
Hide comment
@claudep

claudep Dec 10, 2012

Member

Thanks, fixed in 0cdfa76

Member

claudep replied Dec 10, 2012

Thanks, fixed in 0cdfa76

@dreynolds

This comment has been minimized.

Show comment
Hide comment
@dreynolds

dreynolds Apr 19, 2013

This certainly works for me. Anything we can do to get it committed?

This certainly works for me. Anything we can do to get it committed?

@bhuztez bhuztez referenced this pull request Apr 20, 2013

Closed

Fix management load #866

@Natim

This comment has been minimized.

Show comment
Hide comment
@Natim

Natim Jun 5, 2013

Contributor

Well it works !

Contributor

Natim commented Jun 5, 2013

Well it works !

@rochacbruno

This comment has been minimized.

Show comment
Hide comment
@rochacbruno

rochacbruno Jul 11, 2013

Yes it works! Why isn't accepted yet?

Yes it works! Why isn't accepted yet?

@Natim

This comment has been minimized.

Show comment
Hide comment
@Natim

Natim Jul 12, 2013

Contributor

Please merge. For now to each deployement I have to patch manually in order to see namespace commands.

Contributor

Natim commented Jul 12, 2013

Please merge. For now to each deployement I have to patch manually in order to see namespace commands.

@apollo13

This comment has been minimized.

Show comment
Hide comment
@apollo13

apollo13 Aug 26, 2013

Member

As I noted on the previous PR (#866) I'd like to see:

  • Support for PEP 420
  • Check other locations in Django for the same issues (app template loaders etc come to mind)
Member

apollo13 commented Aug 26, 2013

As I noted on the previous PR (#866) I'd like to see:

  • Support for PEP 420
  • Check other locations in Django for the same issues (app template loaders etc come to mind)
@timgraham

This comment has been minimized.

Show comment
Hide comment
@timgraham

timgraham Sep 18, 2013

Member

Closing this for now in light of @apollo13's comments.

Member

timgraham commented Sep 18, 2013

Closing this for now in light of @apollo13's comments.

@timgraham timgraham closed this Sep 18, 2013

@Natim

This comment has been minimized.

Show comment
Hide comment
@Natim

Natim Sep 19, 2013

Contributor

So it is not fixed !
The code of all the PR is not enough to explain the solution and the problem ?

Contributor

Natim commented Sep 19, 2013

So it is not fixed !
The code of all the PR is not enough to explain the solution and the problem ?

@rochacbruno

This comment has been minimized.

Show comment
Hide comment
@rochacbruno

rochacbruno Sep 19, 2013

Why is it not fixed? I testes this PR, it works well and solves the problem, more than one year, what would be the solution?

Why is it not fixed? I testes this PR, it works well and solves the problem, more than one year, what would be the solution?

@Natim

This comment has been minimized.

Show comment
Hide comment
@Natim

Natim Sep 19, 2013

Contributor

What else do you mean by support for PEP#420 ?
There is no other location bug so far. We are using this fix for a year without problem.

Contributor

Natim commented Sep 19, 2013

What else do you mean by support for PEP#420 ?
There is no other location bug so far. We are using this fix for a year without problem.

@apollo13

This comment has been minimized.

Show comment
Hide comment
@apollo13

apollo13 Sep 19, 2013

Member

@Natim PEP420 gives python 3 implicit namespaces, so this PR should at least have a test to test that those new namespaces work too. Even though the current patch solves your issue, as a core dev our goal is to improve Django as a whole, which for me in that case means to include support for all (common) namespace solutions out there.

@rochacbruno A statement or rather tested support for PEP 420.

As a sidenote: this PR currently includes 12 commits, most of them unrelated to this PR, so that's not really nice to review either… Also this PR has no two pretty much identical codepaths in django/core/management/init.py, which should at least be factored out into a helper function.

Member

apollo13 commented Sep 19, 2013

@Natim PEP420 gives python 3 implicit namespaces, so this PR should at least have a test to test that those new namespaces work too. Even though the current patch solves your issue, as a core dev our goal is to improve Django as a whole, which for me in that case means to include support for all (common) namespace solutions out there.

@rochacbruno A statement or rather tested support for PEP 420.

As a sidenote: this PR currently includes 12 commits, most of them unrelated to this PR, so that's not really nice to review either… Also this PR has no two pretty much identical codepaths in django/core/management/init.py, which should at least be factored out into a helper function.

@rochacbruno

This comment has been minimized.

Show comment
Hide comment
@rochacbruno

rochacbruno Sep 19, 2013

So all we need is to isolate the code related to the namespace issue, create the test for PEP 420 and send another PR?

So all we need is to isolate the code related to the namespace issue, create the test for PEP 420 and send another PR?

@apollo13

This comment has been minimized.

Show comment
Hide comment
@apollo13

apollo13 Sep 19, 2013

Member

Kind of, at least then the PR would be in a reviewable shape.

Member

apollo13 commented Sep 19, 2013

Kind of, at least then the PR would be in a reviewable shape.

@bhuztez

This comment has been minimized.

Show comment
Hide comment
@bhuztez

bhuztez Sep 19, 2013

@apollo13 just one commit, 82f5a71. I guess it is because master has changed after this PR was made.

Django try very hard NOT to import any module when finding management commands, but Django do import app package before finding templates, static files, fixture. So other parts do not have to be patched.

bhuztez commented Sep 19, 2013

@apollo13 just one commit, 82f5a71. I guess it is because master has changed after this PR was made.

Django try very hard NOT to import any module when finding management commands, but Django do import app package before finding templates, static files, fixture. So other parts do not have to be patched.

sztrovacsek pushed a commit to sztrovacsek/django that referenced this pull request Mar 7, 2015

Merge pull request #178 from qba73/qba73
Update attendees_and_learners.rst

nanuxbe pushed a commit to nanuxbe/django that referenced this pull request Jul 2, 2016

Merge pull request #178 from bjacobel/master
Fix a 404 link on documentation index page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment