ft: ARSN-64 sorted set routines #1714

vrancurel · 2022-02-11T03:31:30Z

Large sorted sets management routines implemented with array dichotomies.
No particular suitable external module was found.

An Arsenal tag is not needed at this stage.

bert-e · 2022-02-11T03:31:31Z

Hello vrancurel,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2022-02-11T03:31:35Z

Incorrect fix version

The Fix Version/s in issue ARSN-64 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.1.29

Please check the Fix Version/s of ARSN-64, or the target
branch of this pull request.

codecov-commenter · 2022-02-11T03:33:13Z

Codecov Report

Merging #1714 (90e96c3) into development/8.1 (c95f84e) will increase coverage by 0.17%.
The diff coverage is 91.30%.

@@                 Coverage Diff                 @@
##           development/8.1    #1714      +/-   ##
===================================================
+ Coverage            60.00%   60.17%   +0.17%     
===================================================
  Files                  190      192       +2     
  Lines                12188    12257      +69     
===================================================
+ Hits                  7313     7376      +63     
- Misses                4875     4881       +6

Impacted Files	Coverage Δ
index.js	`100.00% <ø> (ø)`
lib/algos/set/ArrayUtils.js	`91.30% <91.30%> (ø)`
lib/algos/set/SortedSet.js	`91.30% <91.30%> (ø)`

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 c95f84e...90e96c3. Read the comment docs.

tests/unit/set/SortedSet.js

tests/unit/algos/set/ArrayUtils.js

jonathan-gramain · 2022-02-12T00:48:43Z

lib/algos/set/ArrayUtils.js

@@ -0,0 +1,87 @@
+function indexOf(arr, value) {


might be worth unit testing this function thoroughly (correctness, and in case it has corner cases causing infinite loops etc.) if not completely correct it could be a source of nasty bugs later on

jonathan-gramain · 2022-02-12T00:48:52Z

lib/algos/set/ArrayUtils.js

+    return -1;
+}
+
+function indexAtOrBelow(arr, value) {


jonathan-gramain

I would expect the target branch to be development/7.10 since the RING epic is targeted at 8.5.3.

Other than that + a few tests suggestion, good for me

jonathan-gramain · 2022-02-15T23:14:28Z

tests/unit/algos/set/ArrayUtils.js

+        }
+    });
+
+    it('shall find symmetric difference', () => {


Suggested change

it('shall find symmetric difference', () => {

it('symDiff shall find symmetric difference', () => {

jonathan-gramain · 2022-02-15T23:15:32Z

tests/unit/algos/set/ArrayUtils.js

+
+describe('ArrayUtils', () => {
+    it('indexOf', () => {
+        const numOps = 10000;


This test is definitely useful, I'd also add a 0, 1 and 2-item array test for both indexOf and indexAtOrBelow

Large sets management routines implemented with array dichotomies. No particular suitable external module was found.

bert-e · 2022-02-16T22:49:09Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge feature/ARSN-64-sorted-set into
development/7.10
w/8.1/feature/ARSN-64-sorted-set will be merged into development/8.1 (pull request INTEGRATION [PR#1714 > development/8.1] ft: ARSN-64 sorted set routines #1720)

The following branches will NOT be impacted:

development/6.4
development/7.4
stabilization/7.10.2

Follow integration pull requests if you would like to be notified of
build statuses by email.

bert-e · 2022-02-16T22:49:11Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

vrancurel · 2022-02-16T23:00:39Z

/approve

bert-e · 2022-02-16T23:00:48Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/7.10
✔️ development/8.1

The following branches will NOT be impacted:

development/6.4
development/7.4
stabilization/7.10.2

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2022-02-16T23:03:17Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/7.10
✔️ development/8.1

The following branches have NOT changed:

development/6.4
development/7.4
stabilization/7.10.2

Please check the status of the associated issue ARSN-64.

Goodbye vrancurel.

vrancurel requested a review from jonathan-gramain February 11, 2022 03:31

vrancurel requested a review from miniscruff February 11, 2022 03:31

vrancurel force-pushed the feature/ARSN-64-sorted-set branch from d9692c1 to aec4913 Compare February 11, 2022 03:34

miniscruff approved these changes Feb 11, 2022

View reviewed changes

tests/unit/set/SortedSet.js Outdated Show resolved Hide resolved

vrancurel force-pushed the feature/ARSN-64-sorted-set branch from aec4913 to a35f50e Compare February 11, 2022 18:34

jonathan-gramain reviewed Feb 12, 2022

View reviewed changes

vrancurel force-pushed the feature/ARSN-64-sorted-set branch from a35f50e to 90e96c3 Compare February 15, 2022 03:54

jonathan-gramain reviewed Feb 15, 2022

View reviewed changes

vrancurel force-pushed the feature/ARSN-64-sorted-set branch from 90e96c3 to 6050ec1 Compare February 16, 2022 22:10

ft: ARSN-64 sorted set routines

44f37bd

Large sets management routines implemented with array dichotomies. No particular suitable external module was found.

vrancurel force-pushed the feature/ARSN-64-sorted-set branch from 6050ec1 to 44f37bd Compare February 16, 2022 22:44

vrancurel changed the base branch from development/8.1 to development/7.10 February 16, 2022 22:44

scality deleted a comment from bert-e Feb 16, 2022

bert-e mentioned this pull request Feb 16, 2022

INTEGRATION [PR#1714 > development/8.1] ft: ARSN-64 sorted set routines #1720

Merged

scality deleted a comment from bert-e Feb 16, 2022

jonathan-gramain approved these changes Feb 16, 2022

View reviewed changes

bert-e merged commit 29bab6f into development/7.10 Feb 16, 2022

bert-e deleted the feature/ARSN-64-sorted-set branch February 16, 2022 23:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ft: ARSN-64 sorted set routines #1714

ft: ARSN-64 sorted set routines #1714

vrancurel commented Feb 11, 2022 •

edited

Loading

bert-e commented Feb 11, 2022

bert-e commented Feb 11, 2022

codecov-commenter commented Feb 11, 2022 •

edited

Loading

jonathan-gramain Feb 12, 2022

jonathan-gramain Feb 12, 2022

vrancurel Feb 15, 2022

jonathan-gramain left a comment

jonathan-gramain Feb 15, 2022

jonathan-gramain Feb 15, 2022

bert-e commented Feb 16, 2022

bert-e commented Feb 16, 2022

vrancurel commented Feb 16, 2022

bert-e commented Feb 16, 2022

bert-e commented Feb 16, 2022

	it('shall find symmetric difference', () => {
	it('symDiff shall find symmetric difference', () => {

ft: ARSN-64 sorted set routines #1714

ft: ARSN-64 sorted set routines #1714

Conversation

vrancurel commented Feb 11, 2022 • edited Loading

bert-e commented Feb 11, 2022

Hello vrancurel,

bert-e commented Feb 11, 2022

Incorrect fix version

codecov-commenter commented Feb 11, 2022 • edited Loading

Codecov Report

jonathan-gramain Feb 12, 2022

Choose a reason for hiding this comment

jonathan-gramain Feb 12, 2022

Choose a reason for hiding this comment

vrancurel Feb 15, 2022

Choose a reason for hiding this comment

jonathan-gramain left a comment

Choose a reason for hiding this comment

jonathan-gramain Feb 15, 2022

Choose a reason for hiding this comment

jonathan-gramain Feb 15, 2022

Choose a reason for hiding this comment

bert-e commented Feb 16, 2022

Integration data created

bert-e commented Feb 16, 2022

Waiting for approval

vrancurel commented Feb 16, 2022

bert-e commented Feb 16, 2022

In the queue

bert-e commented Feb 16, 2022

vrancurel commented Feb 11, 2022 •

edited

Loading

codecov-commenter commented Feb 11, 2022 •

edited

Loading