Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Split redis in 2 repositories #216

Merged
merged 2 commits into from
Jul 19, 2018

Conversation

melanke
Copy link
Contributor

@melanke melanke commented Jul 12, 2018

Splitting RedisDbRepository into RedisDbBinaryRepository and RedisDbJsonRepository, no need for an if

@codecov
Copy link

codecov bot commented Jul 12, 2018

Codecov Report

Merging #216 into development will decrease coverage by 0.29%.
The diff coverage is 14.66%.

Impacted file tree graph

@@              Coverage Diff               @@
##           development     #216     +/-   ##
==============================================
- Coverage        49.86%   49.57%   -0.3%     
==============================================
  Files              252      254      +2     
  Lines            10760    10854     +94     
==============================================
+ Hits              5366     5381     +15     
- Misses            5394     5473     +79
Impacted Files Coverage Δ
...arp.Persistence.RedisDB/ConfigurationExtensions.cs 0% <0%> (ø) ⬆️
src/NeoSharp.Application/DI/PersistenceModule.cs 0% <0%> (ø) ⬆️
src/NeoSharp.Persistence.RedisDB/RedisDbContext.cs 0% <0%> (ø) ⬆️
...eoSharp.Persistence.RedisDB/RedisDbBinaryConfig.cs 0% <0%> (ø)
.../NeoSharp.Persistence.RedisDB/RedisDbJsonConfig.cs 0% <0%> (ø)
...Sharp.Persistence.RedisDB/RedisDbJsonRepository.cs 18.26% <18.26%> (ø)
...arp.Persistence.RedisDB/RedisDbBinaryRepository.cs 18.26% <33.33%> (ø)

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 849a44f...1b69e48. Read the comment docs.

@shargon
Copy link
Member

shargon commented Jul 12, 2018

check the conflicts with the snowy PR please

@shargon shargon changed the title Splitredisin2repositories Split redis in 2 repositories Jul 12, 2018
@melanke melanke force-pushed the splitredisin2repositories branch 4 times, most recently from 67f6761 to 235bedf Compare July 13, 2018 06:18
@@ -19,7 +19,7 @@ public void Register(IContainerBuilder containerBuilder)
case RedisDbConfig.Provider:
{
containerBuilder.RegisterSingleton<RedisDbConfig>();
containerBuilder.RegisterSingleton<IRepository, RedisDbRepository>();
containerBuilder.RegisterSingleton<IRepository, RedisDbJsonRepository>();
Copy link
Member

Choose a reason for hiding this comment

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

How do you inject RedisDbBinaryRepository ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, replacing RedisDbJsonRepository with RedisDbBinaryRepository, or do it need to change at the runtime?

Copy link
Member

Choose a reason for hiding this comment

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

By configuration, like you choose RocksDb, or Redis

@@ -14,14 +14,14 @@
namespace NeoSharp.Persistence.Redis.Tests
{
[TestClass]
public class UtRedisDbRepository : TestBase
public class UtRedisDbBinaryRepository : TestBase
{
[TestMethod]
public async Task GetIndexHeight_NoValueFound_ReturnsZero()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can join all the unit test of IRepository together, like in a foreach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean to merge all Ut___Repository classes and merge the methods?
Rocks mocking is really different than Redis mocking, so it will probably makes the code less organised and readable. Don't you think?

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Inject the provider according to the configuration file

@melanke melanke force-pushed the splitredisin2repositories branch 2 times, most recently from b83e3e5 to 87d251d Compare July 16, 2018 03:53
@shargon
Copy link
Member

shargon commented Jul 18, 2018

@melanke there are some conflicts here

@shargon shargon merged commit 34431ae into CityOfZion:development Jul 19, 2018
@lock9 lock9 deleted the splitredisin2repositories branch October 29, 2018 14:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants