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 bugs of attribute error 、mongondb duplicate record error、character coding error #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aware-why
Copy link

see code for details

@@ -5,6 +5,7 @@
from w3lib.html import remove_entities
from urlparse import urlparse, urljoin


Copy link
Owner

Choose a reason for hiding this comment

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

多了一个空行

@gnemoug
Copy link
Owner

gnemoug commented Aug 26, 2013

你的pr还是整理一下,再提吧

@aware-why
Copy link
Author

@gnemoug :主要是git的使用不够熟练,所以这个requst就提交成这样了。。。

@gnemoug
Copy link
Owner

gnemoug commented Aug 27, 2013

还有就是代码尽量符合pep8规范,然后我再review!然后push之后,你现在你fork的project上看看commits的diff是否合理,觉得没问题就可以发pr了

@aware-why
Copy link
Author

记得以前在发起pull时可以在页面选择提交哪几个commit,现成貌似不行了

@gnemoug
Copy link
Owner

gnemoug commented Aug 27, 2013

没事,直接在现在基础上改就行,但是以后不要从你的master分支发pr了

@aware-why
Copy link
Author

“不从master分支发pr”什么意思,我对git的概念还不是很清楚,目前我是fork你的之后,然后克隆我fork的到本地,然后将本地的改动提交到fork,然后给你发起pr

@gnemoug
Copy link
Owner

gnemoug commented Aug 27, 2013

你了解一下git的branch的概念吧

@aware-why
Copy link
Author

感觉比svn复杂太多。。。

@gnemoug
Copy link
Owner

gnemoug commented Aug 27, 2013

额,其实我倒是觉得比svn简单很多啊,哈哈

@aware-why
Copy link
Author

刚改了下代码风格,提交pr提示:
Pull request creation failed. Validation failed: A pull request already exists for aware-why:master.

是不是这个pr得先取消了,才能再次提交?

@gnemoug
Copy link
Owner

gnemoug commented Aug 27, 2013

不用提pr,你的修改已经同步到你原来提的pr中了

2013/8/27 aware-why [email protected]

刚改了下代码风格,提交pr提示:
Pull request creation failed. Validation failed: A pull request already
exists for aware-why:master.

是不是这个pr得先取消了,才能再次提交?


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-23312966
.

@@ -126,8 +126,8 @@ def process_item(self, item, spider):
"""
custom process_item func,so it will manage the Request result.
"""

info = self.spiderinfo[spider]
log.msg("FUCK*** come here meida_naem=%s" % self.MEDIA_NAME, level=log.DEBUG, spider=spider)
Copy link
Owner

Choose a reason for hiding this comment

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

FUCK 。。。这就不要了吧

Copy link
Author

Choose a reason for hiding this comment

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

On Tue, Aug 27, 2013 at 12:54 PM, gnemoug [email protected] wrote:

In woaidu_crawler/woaidu_crawler/pipelines/mongodb_book_file.py:

@@ -126,8 +126,8 @@ def process_item(self, item, spider):
"""
custom process_item func,so it will manage the Request result.

"""

  •    info = self.spiderinfo[spider]
    
  •    log.msg("FUCK**\* come here meida_naem=%s" % self.MEDIA_NAME, level=log.DEBUG, spider=spider)
    

FUCK 。。。这就不要了吧


Reply to this email directly or view it on GitHubhttps://github.com//pull/5/files#r5997204
.

这个不是已经去掉了吗?

@gnemoug
Copy link
Owner

gnemoug commented Aug 28, 2013

恩,等抽空我测试下,再merge吧!

@aware-why
Copy link
Author

On Wed, Aug 28, 2013 at 8:20 AM, gnemoug [email protected] wrote:

恩,等抽空我测试下,再merge吧!


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-23381620
.

你应该是个python大牛吧,我刚接触不久,在qihoo360工作。
你呢?

@gnemoug
Copy link
Owner

gnemoug commented Aug 28, 2013

。。。。我刚接触编程2年,大三的学生,在找工作中,目前在豆瓣实习,好像离360很近的说。。。

@gnemoug
Copy link
Owner

gnemoug commented Aug 28, 2013

360好像不用python吧!!!

@aware-why
Copy link
Author

On Wed, Aug 28, 2013 at 12:22 PM, gnemoug [email protected] wrote:

360好像不用python吧!!!


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-23390142
.

用,后台开发用到。
我看你在github上参与的项目很多,看来很热爱python

@gnemoug
Copy link
Owner

gnemoug commented Aug 28, 2013

给个联系方式?我的[email protected]

@aware-why
Copy link
Author

On Wed, Aug 28, 2013 at 6:17 PM, gnemoug [email protected] wrote:

¸ø¸öÁªÏµ·½Ê½£¿ÎÒµÄ[email protected]

¡ª
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-23404066
.

[email protected]

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.

2 participants