search

Giorgi / DuckDB.NET

Bindings and ADO.NET Provider for DuckDB
https://duckdb.net
MIT License
338 stars 61 forks source link

Refactor/no boxing #155

Closed Seddryck closed 8 months ago

coveralls commented 8 months ago

Pull Request Test Coverage Report for Build 6699497358

Changes Missing Coverage Covered Lines Changed/Added Lines %
DuckDB.NET.Data/TypeHandlers/HugeIntTypeHandler.cs 7 8 87.5%
DuckDB.NET.Data/TypeHandlers/StructTypeHandler.cs 28 29 96.55%
DuckDB.NET.Data/TypeHandlers/TimestampTypeHandler.cs 8 9 88.89%
DuckDB.NET.Data/TypeHandlers/TypeHandlerFactory.cs 25 26 96.15%
DuckDB.NET.Data/TypeHandlers/VarcharTypeHandler.cs 11 12 91.67%
DuckDB.NET.Data/TypeHandlers/BaseTypeHandler.cs 41 43 95.35%
DuckDB.NET.Data/TypeHandlers/IntervalTypeHandler.cs 6 8 75.0%
DuckDB.NET.Data/TypeHandlers/ListTypeHandler.cs 67 69 97.1%
DuckDB.NET.Data/TypeHandlers/NumericTypeHandler.cs 16 18 88.89%
DuckDB.NET.Data/DuckDBDataReader.cs 18 21 85.71%
<!-- Total: 354 394 89.85% -->
Files with Coverage Reduction New Missed Lines %
DuckDB.NET.Data/DuckDBDataReader.cs 2 87.64%
DuckDB.NET.Bindings/DuckDBNativeObjects.cs 3 73.26%
<!-- Total: 5 -->
Totals Coverage Status
Change from base Build 6695276657: -0.8%
Covered Lines: 1084
Relevant Lines: 1236
💛 - Coveralls
Seddryck commented 8 months ago

Sounds that the CICD pipeline is fixed for PR (#154)

Seddryck commented 8 months ago

I could add tests to reach 100% of code coverage, it's mostly the same line implying a conversion that is not hit by current tests. But more fundamentally, the point is that reading and converting are two different concerns and basically we should have two classes, which will let the developer deploy his/her own converters. You could imagine reading a varchar as a string {x=120;y=235} and convert it as Point. That would require a bit of additional features in the factory to let developers register their own couple of reader/converter but will bring additional power. It could be also two separate features.

Seddryck commented 8 months ago

Closing a PR without comment?

Giorgi commented 8 months ago

Looks like the PR was closed automatically when I merged and deleted Struct-Support into the develop branch.

As for the PR itself, I liked your suggestions and ideas but didn't like some of the little details so I decided to continue my refactoring process (which was largely influenced and inspired by your ideas).

Even though I'm not accepting your PR, please don't think that I didn't find your suggestions valuable, quite the contrary. You definitely contributed to the project by sending this PR!