Skip to content

Conversation

@sirtimid
Copy link
Contributor

@sirtimid sirtimid commented Mar 28, 2025

Closes #454

Refactor the kernel store by breaking the current big file into smaller, easier-to-manage pieces.
Also added lots of missing unit tests.

@sirtimid sirtimid requested a review from a team as a code owner March 28, 2025 13:52
@sirtimid sirtimid changed the title refactor: kernel-store refactor: kernel-store into smaller units Mar 28, 2025
@sirtimid sirtimid self-assigned this Mar 28, 2025
@grypez grypez self-assigned this Mar 31, 2025
@grypez grypez self-requested a review March 31, 2025 21:46
@grypez grypez removed their assignment Mar 31, 2025
Comment on lines 13 to 18
it('returns the index of the first key greater than the search key', () => {
const arr = ['a', 'c', 'e', 'g', 'i'];
expect(keySearch(arr, 'b')).toBe(1);
expect(keySearch(arr, 'd')).toBe(1);
expect(keySearch(arr, 'f')).toBe(-1);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem right.

Suggested change
it('returns the index of the first key greater than the search key', () => {
const arr = ['a', 'c', 'e', 'g', 'i'];
expect(keySearch(arr, 'b')).toBe(1);
expect(keySearch(arr, 'd')).toBe(1);
expect(keySearch(arr, 'f')).toBe(-1);
});
it('returns the index of the first key greater than the search key', () => {
const arr = ['a', 'c', 'e', 'g', 'i'];
expect(keySearch(arr, 'b')).toBe(1);
expect(keySearch(arr, 'd')).toBe(2);
expect(keySearch(arr, 'z')).toBe(-1);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I fixed the algorithm 3955435

@sirtimid sirtimid force-pushed the sirtimid/refactor-kernel-store branch from d0140df to 3955435 Compare April 1, 2025 12:38
@sirtimid sirtimid requested a review from grypez April 1, 2025 12:38
@sirtimid sirtimid enabled auto-merge (squash) April 1, 2025 12:39
Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

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

LGTM

@sirtimid sirtimid merged commit e371f6b into main Apr 1, 2025
21 checks passed
@sirtimid sirtimid deleted the sirtimid/refactor-kernel-store branch April 1, 2025 15:01
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.

Break kernel-store into smaller pieces with unit tests

3 participants