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.

Sign up for GitHub

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

Jump to bottom

misc code improvements #91

Closed
wants to merge 26 commits into from
Closed

misc code improvements #91

wants to merge 26 commits into from

Conversation

tegk
Copy link

@tegk tegk commented Oct 17, 2019 •
edited by manishrjain
Loading

I reviewed the code and changed miscellaneous problems.

This change is Reviewable

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

tegk seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

tegk and others added 15 commits October 17, 2019 14:42
@tegk
TestMain should call os.Exit to set exit code otherwise its always ze…
2310823 
…ro even if it fails!
@tegk
error strings should not end with punctation since they are usually p…
df38d06 
…rinted following other context
@tegk
should omit type time.Duration from declaration of var wait; it will …
954cef3 
…be inferred from the right-hand side
@tegk
acronyms should be capitalized
fd29622
@tegk
increment with ++ is the way to go
54cacf5
@tegk
trailing whitespace removed
8baf1cb
@tegk
single case switch should be if statement
dfefa25
@tegk
replace else {if cond {}} with else if cond {}
098e498
@tegk
gofmt -s
64e5834
@tegk
Merge branch 'master' of https://github.com/tegk/ristretto
40b84bd
@tegk
single case switch should be if statement
e9795b2
@tegk
replace else {if cond {}} with else if cond {}
1be4690
@tegk
gofmt -s
569769d
@tegk
Merge branch 'master' of https://github.com/tegk/ristretto
1be40e1
@tegk
Merge branch 'master' of https://github.com/tegk/ristretto
aeaab35
@tegk
Copy link
Author

tegk commented Oct 17, 2019 •
edited
Loading

I rebased and signed the CLA but still does not work. Could somebody help or manually approve that i signed the CLA.

andersfylling
andersfylling approved these changes Oct 21, 2019
View reviewed changes
sketch.go Outdated Show resolved Hide resolved
z/bbloom_test.go Outdated Show resolved Hide resolved
@karlmcguire
Copy link
Contributor

@tegk Thanks for the PR. Small cosmetic things matter, so I appreciate it.

The CLA check is failing because you used two emails for the commits in this PR. You probably don't have access to [email protected], so I will check if there is a way to override that.

@martinmr
Copy link
Contributor

Closing in favor of #126 to handle conflicts.

I didn't include the changes to add periods at the end of error messages (the golang convention is to not include them) and the change of Id to ID in variable names since we don't follow that convention in our other golang projects.

@tegk Thanks for your contribution.

@martinmr martinmr closed this Jan 18, 2020
@martinmr martinmr mentioned this pull request Jan 18, 2020
Merged
@jarifibrahim jarifibrahim mentioned this pull request Jun 30, 2020
Merged
jarifibrahim pushed a commit to dgraph-io/badger that referenced this pull request Jun 30, 2020
Update ristretto to commit f66de99 (#1391)
09dfa66 
The update brings following changes from ristretto into badger

```
f66de99 Improve handling of updated items (dgraph-io/ristretto#168)
aec7994 Fix flaky TestCacheDel test (dgraph-io/ristretto#169)
a093fe6 Add benchmark plots to the project repo (dgraph-io/ristretto#166)
49dc42c Add Anurag as codeowner (dgraph-io/ristretto#158)
7a3f2d3 z: use MemHashString and xxhash.Sum64String (dgraph-io/ristretto#153)
9c31bb2 Check conflict key before updating expiration map. (dgraph-io/ristretto#154)
62cb731 Fix race condition in Cache.Clear (dgraph-io/ristretto#133)
9af1934 Docs and whitespace changes for readability. (dgraph-io/ristretto#143)
dbc185e Add changelog. (dgraph-io/ristretto#142)
ff325ad Remove key from policy after TTL eviction (dgraph-io/ristretto#130)
2dd5ff5 Use the require library in all the tests. (dgraph-io/ristretto#128)
7c48141 Use require in all tests in cache_test.go (dgraph-io/ristretto#127)
51e97ad Sets with TTL (dgraph-io/ristretto#122)
d3e7c37 Add benchmarks for math.rand and fastrand (dgraph-io/ristretto#118)
593823e Integrate fixes from PR dgraph-io/ristretto#91. (dgraph-io/ristretto#126)
29b4dd7 Fix comments. (dgraph-io/ristretto#123)
ddf345c Removed workflows directory. (dgraph-io/ristretto#124)
eb104d0 Add martinmr to CODEOWNERS file. (dgraph-io/ristretto#125)
```
jarifibrahim pushed a commit to dgraph-io/badger that referenced this pull request Jul 6, 2020
Update ristretto to commit f66de99 (#1391)
25fd0ef 
The update brings following changes from ristretto into badger

```
f66de99 Improve handling of updated items (dgraph-io/ristretto#168)
aec7994 Fix flaky TestCacheDel test (dgraph-io/ristretto#169)
a093fe6 Add benchmark plots to the project repo (dgraph-io/ristretto#166)
49dc42c Add Anurag as codeowner (dgraph-io/ristretto#158)
7a3f2d3 z: use MemHashString and xxhash.Sum64String (dgraph-io/ristretto#153)
9c31bb2 Check conflict key before updating expiration map. (dgraph-io/ristretto#154)
62cb731 Fix race condition in Cache.Clear (dgraph-io/ristretto#133)
9af1934 Docs and whitespace changes for readability. (dgraph-io/ristretto#143)
dbc185e Add changelog. (dgraph-io/ristretto#142)
ff325ad Remove key from policy after TTL eviction (dgraph-io/ristretto#130)
2dd5ff5 Use the require library in all the tests. (dgraph-io/ristretto#128)
7c48141 Use require in all tests in cache_test.go (dgraph-io/ristretto#127)
51e97ad Sets with TTL (dgraph-io/ristretto#122)
d3e7c37 Add benchmarks for math.rand and fastrand (dgraph-io/ristretto#118)
593823e Integrate fixes from PR dgraph-io/ristretto#91. (dgraph-io/ristretto#126)
29b4dd7 Fix comments. (dgraph-io/ristretto#123)
ddf345c Removed workflows directory. (dgraph-io/ristretto#124)
eb104d0 Add martinmr to CODEOWNERS file. (dgraph-io/ristretto#125)
```

(cherry picked from commit 09dfa66)
jarifibrahim pushed a commit to dgraph-io/badger that referenced this pull request Oct 2, 2020
Update ristretto to commit f66de99 (#1391)
0e9748b 
The update brings following changes from ristretto into badger

```
f66de99 Improve handling of updated items (dgraph-io/ristretto#168)
aec7994 Fix flaky TestCacheDel test (dgraph-io/ristretto#169)
a093fe6 Add benchmark plots to the project repo (dgraph-io/ristretto#166)
49dc42c Add Anurag as codeowner (dgraph-io/ristretto#158)
7a3f2d3 z: use MemHashString and xxhash.Sum64String (dgraph-io/ristretto#153)
9c31bb2 Check conflict key before updating expiration map. (dgraph-io/ristretto#154)
62cb731 Fix race condition in Cache.Clear (dgraph-io/ristretto#133)
9af1934 Docs and whitespace changes for readability. (dgraph-io/ristretto#143)
dbc185e Add changelog. (dgraph-io/ristretto#142)
ff325ad Remove key from policy after TTL eviction (dgraph-io/ristretto#130)
2dd5ff5 Use the require library in all the tests. (dgraph-io/ristretto#128)
7c48141 Use require in all tests in cache_test.go (dgraph-io/ristretto#127)
51e97ad Sets with TTL (dgraph-io/ristretto#122)
d3e7c37 Add benchmarks for math.rand and fastrand (dgraph-io/ristretto#118)
593823e Integrate fixes from PR dgraph-io/ristretto#91. (dgraph-io/ristretto#126)
29b4dd7 Fix comments. (dgraph-io/ristretto#123)
ddf345c Removed workflows directory. (dgraph-io/ristretto#124)
eb104d0 Add martinmr to CODEOWNERS file. (dgraph-io/ristretto#125)
```
manishrjain pushed a commit to outcaste-io/outserv that referenced this pull request Jul 6, 2022
Update ristretto to commit f66de99 (#1391)
cf51ce1 
The update brings following changes from ristretto into badger

```
f66de99 Improve handling of updated items (dgraph-io/ristretto#168)
aec7994 Fix flaky TestCacheDel test (dgraph-io/ristretto#169)
a093fe6 Add benchmark plots to the project repo (dgraph-io/ristretto#166)
49dc42c Add Anurag as codeowner (dgraph-io/ristretto#158)
7a3f2d3 z: use MemHashString and xxhash.Sum64String (dgraph-io/ristretto#153)
9c31bb2 Check conflict key before updating expiration map. (dgraph-io/ristretto#154)
62cb731 Fix race condition in Cache.Clear (dgraph-io/ristretto#133)
9af1934 Docs and whitespace changes for readability. (dgraph-io/ristretto#143)
dbc185e Add changelog. (dgraph-io/ristretto#142)
ff325ad Remove key from policy after TTL eviction (dgraph-io/ristretto#130)
2dd5ff5 Use the require library in all the tests. (dgraph-io/ristretto#128)
7c48141 Use require in all tests in cache_test.go (dgraph-io/ristretto#127)
51e97ad Sets with TTL (dgraph-io/ristretto#122)
d3e7c37 Add benchmarks for math.rand and fastrand (dgraph-io/ristretto#118)
593823e Integrate fixes from PR dgraph-io/ristretto#91. (dgraph-io/ristretto#126)
29b4dd7 Fix comments. (dgraph-io/ristretto#123)
ddf345c Removed workflows directory. (dgraph-io/ristretto#124)
eb104d0 Add martinmr to CODEOWNERS file. (dgraph-io/ristretto#125)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andersfylling andersfylling andersfylling approved these changes

@karlmcguire karlmcguire karlmcguire approved these changes

@jarifibrahim jarifibrahim Awaiting requested review from jarifibrahim

@manishrjain manishrjain Awaiting requested review from manishrjain

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tegk @CLAassistant @karlmcguire @martinmr @andersfylling