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

#69: Added more tests for SET command #399

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

#69: Added more tests for SET command #399

wants to merge 4 commits into from
+83 −0

Conversation

nihcas700
Copy link

@nihcas700 nihcas700 commented Aug 23, 2024 •
edited by JyotinderSingh
Loading

Issue #69

@nihcas700 nihcas700 mentioned this pull request Aug 23, 2024
Open
lucifercr07
lucifercr07 requested changes Aug 23, 2024
View reviewed changes
tests/set_test.go Outdated Show resolved Hide resolved
tests/utils.go Outdated Show resolved Hide resolved
@JyotinderSingh
Copy link
Collaborator

Please resolve the merge conflicts.

@JyotinderSingh JyotinderSingh changed the title [69] Added more tests for SET command #69: Added more tests for SET command Aug 25, 2024
JyotinderSingh
JyotinderSingh requested changes Aug 25, 2024
View reviewed changes
Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

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

Thanks for these changes @nihcas700! I have left a few comments.

tests/setup.go Outdated Show resolved Hide resolved
tests/set_test.go Outdated Show resolved Hide resolved
testutils/random_nums.go Outdated Show resolved Hide resolved
@JyotinderSingh
Copy link
Collaborator

There are some build failures, could you please take a look?

@AshwinKul28
Copy link
Contributor

@nihcas700 Can you please have a look at the CI failures?

@JyotinderSingh
Copy link
Collaborator

Hi @nihcas700, are you actively working on this?

@nihcas700
Copy link
Author

Hi @JyotinderSingh : Sorry, I got occupied with something else, would get it done by the upcoming weekend.

@AshwinKul28
Copy link
Contributor

Hey @nihcas700 , Any further updates on this? If you have any blockers, please discuss it over here or on the discord. Let's try to merge the PR and close out because a lot many changes are getting merged into master, and every time it would be difficult to resolve the conflicts and rebase with the master for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JyotinderSingh JyotinderSingh JyotinderSingh requested changes

@lucifercr07 lucifercr07 lucifercr07 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nihcas700 @JyotinderSingh @AshwinKul28 @lucifercr07