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

修复断点续传功能bug #1931

Merged
merged 2 commits into from Nov 20, 2019
Merged

Conversation

gaolizheng
Copy link
Contributor

@gaolizheng gaolizheng commented Nov 13, 2019

修复断点续传功能bug

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 13, 2019

感谢贡献,有论坛id么,我们可以给你一个论坛的头衔

@gaolizheng
Copy link
Contributor Author

@gaolizheng gaolizheng commented Nov 13, 2019

论坛id是gaolizheng,备感荣幸 @holycanvas

@jareguo
Copy link
Member

@jareguo jareguo commented Nov 13, 2019

谢啦。已标注为“Cocos 开源贡献者”

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 13, 2019

是我们的荣幸才是

@holycanvas holycanvas requested a review from minggo Nov 15, 2019
@minggo minggo requested review from PatriceJiang and removed request for minggo Nov 15, 2019
@@ -190,7 +190,7 @@ public void onResponse(Call call, Response response) throws IOException {

try {

if(response.code() != 200) {
if(response.code() != 200 && response.code() != 206) {
Copy link
Contributor

@PatriceJiang PatriceJiang Nov 15, 2019

Choose a reason for hiding this comment

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

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
根据规范, 200~299 内的状态码都可以当作下载成功。 建议把条件放宽为

response.code() >= 200 && response.code() <= 206

代码204,205也是有可能出现的。

Copy link
Contributor Author

@gaolizheng gaolizheng Nov 18, 2019

Choose a reason for hiding this comment

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

204 No Content
服务器成功处理了请求,但不需要返回任何实体内容
205 Reset Content
服务器成功处理了请求,且没有返回任何内容。

个人感觉针对下载文件这个需求来说,出现204或者205应该都是开发者不希望的情况吧

Copy link
Contributor

@PatriceJiang PatriceJiang Nov 18, 2019

Choose a reason for hiding this comment

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

按照规范 204/205 也算作“成功”的状态, 具体是否会“出现” 取决于服务器的实现。我认为尽量按照规范处理是比较合理的。

@gaolizheng
Copy link
Contributor Author

@gaolizheng gaolizheng commented Nov 18, 2019

已经修改成状态码200-206均视为成功

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 19, 2019

你好,能改为用英文写commit的信息么,pr里面可以用中文写,commit里面最好不出现中文,毕竟开源项目

@gaolizheng
Copy link
Contributor Author

@gaolizheng gaolizheng commented Nov 19, 2019

你好,能改为用英文写commit的信息么,pr里面可以用中文写,commit里面最好不出现中文,毕竟开源项目

可以用英文,但是我已经push上去的commit没法改了吧,我该怎么操作😢

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 19, 2019

可以改的,现在你本地改了后用 push -f 强推就可以覆盖你之前的commit

@gaolizheng
Copy link
Contributor Author

@gaolizheng gaolizheng commented Nov 19, 2019

可以改的,现在你本地改了后用 push -f 强推就可以覆盖你之前的commit

改了,但是好像只能改最后一次的commit信息😢

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 19, 2019

git rebase 命令可以重新写注释信息,然后再强推就行了

@gaolizheng
Copy link
Contributor Author

@gaolizheng gaolizheng commented Nov 20, 2019

git rebase 命令可以重新写注释信息,然后再强推就行了

学到了,已改。

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 20, 2019

感谢你的贡献

@holycanvas holycanvas merged commit 47ca5b9 into cocos:v2.2.1-release Nov 20, 2019
1 check passed
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.

None yet

4 participants