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.

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

pipeline examples #75

Merged
merged 13 commits into from
Feb 5, 2023
Merged

Conversation

Jeevananthan-23
Copy link
Contributor

@Jeevananthan-23 Jeevananthan-23 commented Jan 30, 2023

Closes #61
Closes #62
Closes #65

@Jeevananthan-23 Jeevananthan-23 mentioned this pull request Jan 30, 2023
6 tasks
@shacharPash
Copy link
Contributor

ready for review?
because it's still draft

Copy link
Contributor

@shacharPash shacharPash left a comment

Choose a reason for hiding this comment

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

Great job man!
I wrote some small comments that I think should be changed.
Thanks :)

chayim and others added 2 commits January 30, 2023 17:09
* Adding a README to the nuget package

* readme path

* fixing readme path
@Jeevananthan-23 Jeevananthan-23 marked this pull request as ready for review January 31, 2023 13:57
```

## Search
Create the schema to index first and age as a numeric field
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't quite understand this sentence..
Did you mean to write something like this?
Create the schema to index name as text field, age as a numeric field and city as tag field.

Copy link
Contributor

@shacharPash shacharPash left a comment

Choose a reason for hiding this comment

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

hey @Jeevananthan-23, thanks again :)
I added some comments.
There are still two things I wrote in the previous review.

  1. You should also give an example of creating a pipeline in this way: var pipeline = new Pipeline(db);, Pipeline can get IDatabase or IConnectionMultiplexer and I want to show this 2 option in the examples.
  2. You need to add Assertions to the tests to make sure that what you wrote in the examples really gives the desired result.

@shacharPash
Copy link
Contributor

@Jeevananthan-23
The tests don't pass for some reason, I tried to fix them but there is still a problem.
if you understand why they fail try to fix it, I will only be able to work on it on Sunday probably.
thanks for your wrok :)

@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2023

Codecov Report

Base: 93.32% // Head: 93.38% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (0803d30) compared to base (6a1503b).
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff              @@
##           examples      #75      +/-   ##
============================================
+ Coverage     93.32%   93.38%   +0.06%     
============================================
  Files            76       76              
  Lines          4555     4551       -4     
  Branches        422      422              
============================================
- Hits           4251     4250       -1     
+ Misses          181      179       -2     
+ Partials        123      122       -1     
Impacted Files Coverage Δ
src/NRedisStack/Pipeline.cs 100.00% <ø> (ø)
src/NRedisStack/Search/Document.cs 86.00% <0.00%> (+6.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Jeevananthan-23
Copy link
Contributor Author

@Jeevananthan-23 The tests don't pass for some reason, I tried to fix them but there is still a problem. if you understand why they fail try to fix it, I will only be able to work on it on Sunday probably. thanks for your wrok :)

@shacharPash this might be the long-running pipelining and it may occur often.

@shacharPash shacharPash requested review from chayim and removed request for chayim February 5, 2023 15:34
@shacharPash shacharPash merged commit 65cfcbe into redis:examples Feb 5, 2023
@shacharPash shacharPash added the documentation Improvements or additions to documentation label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants