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

[MRG] Fix startproject project name #817

Merged
merged 2 commits into from Aug 4, 2014

Conversation

@nramirezuy
Copy link
Contributor

@nramirezuy nramirezuy commented Jul 25, 2014

#434 fixes.

Add project name validation

@nramirezuy nramirezuy changed the title Fix startproject project name [MRG] Fix startproject project name Jul 31, 2014
def _module_exists(module_name):
try:
import_module(module_name)
return True

This comment has been minimized.

@dangra

dangra Aug 1, 2014
Member

shouldn't it return 'False' when the module is importable? IMO the idea is that a project can't get the name of an existent python package.

This comment has been minimized.

else:
return True

self.exitcode = 1

This comment has been minimized.

@dangra

dangra Aug 1, 2014
Member

Although writing to stdout can be considered a side-effect, setting self.exitcode here looks bad to me. the exitcode should be set by the caller instead.

print(" cd %s" % project_name)
print(" scrapy genspider example example.com")

if self._is_valid_name(project_name):

This comment has been minimized.

@dangra

dangra Aug 1, 2014
Member

I would use the inverse conditional here:

if self._is_invalid_name(project_name):
    self.exitcode = 1
    return

# Create the project below
....
Nuno Maximiano and others added 2 commits Oct 18, 2013
dangra added a commit that referenced this pull request Aug 4, 2014
[MRG] Fix startproject project name
@dangra dangra merged commit 511a269 into scrapy:master Aug 4, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@timschwab
Copy link

@timschwab timschwab commented Jun 16, 2015

Correct me if I am wrong, but this code is meant to prevent users from creating projects with module names. Well I recently started using Scrapy and no errors are thrown on creation when I type scrapy startproject test. The errors start appearing later, the same way as the original issue, #428.

@dangra
Copy link
Member

@dangra dangra commented Jun 16, 2015

@timschwab what scrapy version are you using?

@timschwab
Copy link

@timschwab timschwab commented Jun 16, 2015

Scrapy 0.24.6

@dangra
Copy link
Member

@dangra dangra commented Jun 16, 2015

@timschwab: that change is not part of 0.24 branch. You can install 1.0.0rc3 from pypi, it is going to be released this/next week as final: pip install Scrapy==1.0.0rc3

@timschwab
Copy link

@timschwab timschwab commented Jun 16, 2015

@dangra: gotcha. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants