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

Fix bug for ".local" host name #878

Merged
merged 2 commits into from Sep 20, 2014
Merged

Fix bug for ".local" host name #878

merged 2 commits into from Sep 20, 2014

Conversation

@andrewshir
Copy link
Contributor

@andrewshir andrewshir commented Sep 6, 2014

It's necessary to put new list member in square brackets (i.e. create new list) to merge lists properly, otherwise we will get result list with character elements instead of string element.

It's necessary to put new list member in squared brackets (i.e. create new list) to merge lists properly, otherwise we will get result list with character elements instead of string element.
@nramirezuy
Copy link
Contributor

@nramirezuy nramirezuy commented Sep 8, 2014

+1

>>> hosts = []
>>> hosts += 'req_host' + '.local'
>>> hosts
['r', 'e', 'q', '_', 'h', 'o', 's', 't', '.', 'l', 'o', 'c', 'a', 'l']
>>> hosts = []
>>> hosts += ['req_host' + '.local']
>>> hosts
['req_host.local']
@pablohoffman
Copy link
Member

@pablohoffman pablohoffman commented Sep 8, 2014

Thanks for the fix. I'd prefer to merge it with a test, if possible. Is it easy to add one?

@andrewshir
Copy link
Contributor Author

@andrewshir andrewshir commented Sep 16, 2014

Yes, sure. Please review.

@kmike
Copy link
Member

@kmike kmike commented Sep 20, 2014

Thanks @andrewshir!

kmike added a commit that referenced this pull request Sep 20, 2014
Fix bug for ".local" host name
@kmike kmike merged commit c543fe6 into scrapy:master Sep 20, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
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

4 participants
You can’t perform that action at this time.