Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

Conversation

@wangforthinker
Copy link
Contributor

Signed-off-by: allen.wq allen.wq@alipay.com

Ⅰ. Describe what this PR did

Add the lru queue and mock file server. It may be used in some test.

Ⅱ. Does this pull request fix one issue?

NONE.

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Already add it.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@wangforthinker wangforthinker force-pushed the feat/add-lru-queue-and-mockfileserver-prepare-pr branch from 56f4403 to 40b88a7 Compare April 10, 2020 04:07
@hhhhsdxxxx
Copy link

lgtm!
Reviewed-by henry.hj@antfin.com

@wangforthinker wangforthinker force-pushed the feat/add-lru-queue-and-mockfileserver-prepare-pr branch 2 times, most recently from 2e13431 to c4e6fb0 Compare April 10, 2020 07:46
@codecov-io
Copy link

Codecov Report

Merging #1275 into master will increase coverage by 0.78%.
The diff coverage is 84.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1275      +/-   ##
==========================================
+ Coverage   48.96%   49.74%   +0.78%     
==========================================
  Files         119      120       +1     
  Lines        7763     7936     +173     
==========================================
+ Hits         3801     3948     +147     
- Misses       3647     3665      +18     
- Partials      315      323       +8     
Impacted Files Coverage Δ
dfget/core/helper/test_helper.go 41.93% <76.47%> (+41.93%) ⬆️
pkg/queue/lru_queue.go 97.18% <97.18%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7856ccd...c4e6fb0. Read the comment docs.

data interface{}
}

type LRUQueue struct {
Copy link
Member

Choose a reason for hiding this comment

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

add comments for export statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

return nil, fmt.Errorf("resp code %d", resp.StatusCode)
}

defer resp.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid that you should move this defer in front of the 400 judgement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

pkg/queue/lru_queue.go Show resolved Hide resolved
pkg/queue/lru_queue.go Show resolved Hide resolved
pkg/queue/lru_queue.go Outdated Show resolved Hide resolved
Signed-off-by: allen.wq <allen.wq@alipay.com>
@wangforthinker wangforthinker force-pushed the feat/add-lru-queue-and-mockfileserver-prepare-pr branch from c4e6fb0 to b3222f5 Compare April 10, 2020 09:15

result[index] = item.Value.(*cQElementData).data
index++
if index >= count {
Copy link
Member

Choose a reason for hiding this comment

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

The index should be less the length of the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if list is traversed to the end, the loop will be break by the case "item == nil"

Copy link
Member

@lowzj lowzj left a comment

Choose a reason for hiding this comment

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

LGTM

@lowzj lowzj merged commit 19f1613 into dragonflyoss:master Apr 14, 2020
@wangforthinker wangforthinker deleted the feat/add-lru-queue-and-mockfileserver-prepare-pr branch April 23, 2020 03:18
sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022
Signed-off-by: Jim Ma <majinjing3@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.