Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor(clustering) dpcp config clean #8880
refactor(clustering) dpcp config clean #8880
Changes from 48 commits
31152d1
283a966
2595d8e
3179b83
dd506af
cb924a0
a1f148d
8fa4a6a
63b3edd
0f1e9f0
65a3f9a
c122969
2284eb2
8c4fb09
6a48b69
68925f9
3ec4be2
8709d5d
486c70f
b8ed36f
61075ae
c060981
66abd81
21f79b9
3b04364
16dee27
4da6404
0a8bf89
de66e7a
c5be4cf
d5436d7
e4b74d7
9d4169a
ea83d84
5eba885
bd244b6
329688e
4b8895d
69e1204
66d10d7
7affc3e
07fd555
79a275d
9a2bc91
031c18e
310a3b5
18f7fb5
36e6540
69a003d
7c98016
e4c3543
4f0adb1
a615cbb
0f8a2eb
436cb82
5a25d4d
e9cb7d1
0155ea8
fe2d337
de945de
288e667
6d5aa75
cddc8e2
4ea4949
092ad86
6ea52d0
c5bb0cd
7287710
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there plans to fix this off-by-one error?
on every
j==n*100
, replaces all values ino
with a hash of the first 100 and setsi
to 1, so the next value (number 101) will go ono[2]
. So whenj==200
, there will be 101 values ino
(the last hash plus 100 values more) but it will again use only 100 values for the next hash, so it drops the hashes values of items number 201, 301, 401.... etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems true. Shall we fix it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can use
local i = 1
andnew_tab(count < 100 and count +1 or 101, 0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. mentioned it just that it can be discussed. probably in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think perhaps we can not change it because of the compatibility of old version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the hash method has changed several times already. there's no compatibility issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's fix the off-by-one error on this PR then then. Another PR will make everything take even longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will force us to modify all the hashes on the test, which is a pain.
New directive: Do it only if it can be done quickly on this PR. We have lived with the problem so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do it later, now we should finish refactoring first.