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

feat: Rework GetCollection/SchemaByFoo funcs into single #2319

Merged

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #2008

Description

Reworks the GetCollection/SchemaByFoo funcs into single GetCollections and GetSchemas funcs.

Commits are mostly clean, and might be easier to review from everything at once, but watch out for a pair of small fixup commits I left in the middle (sorry).

@AndrewSisley AndrewSisley added area/collections Related to the collections system code quality Related to improving code quality labels Feb 16, 2024
@AndrewSisley AndrewSisley added this to the DefraDB v0.10 milestone Feb 16, 2024
@AndrewSisley AndrewSisley requested a review from a team February 16, 2024 15:35
@AndrewSisley AndrewSisley self-assigned this Feb 16, 2024
@AndrewSisley AndrewSisley changed the title feat: Rework GetCollection/SchemaByFoo funcs into single func feat: Rework GetCollection/SchemaByFoo funcs into single Feb 16, 2024
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (cdb8ff8) 74.29% compared to head (53f3f10) 74.81%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2319      +/-   ##
===========================================
+ Coverage    74.29%   74.81%   +0.52%     
===========================================
  Files          256      256              
  Lines        25373    25254     -119     
===========================================
+ Hits         18849    18892      +43     
+ Misses        5223     5076     -147     
+ Partials      1301     1286      -15     
Flag Coverage Δ
all-tests 74.81% <80.08%> (+0.52%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
cli/schema_describe.go 100.00% <100.00%> (+16.07%) ⬆️
db/backup.go 59.64% <100.00%> (ø)
db/txn_db.go 62.71% <100.00%> (+8.36%) ⬆️
net/peer_collection.go 58.59% <100.00%> (+3.51%) ⬆️
net/peer_replicator.go 77.12% <100.00%> (ø)
net/server.go 71.30% <100.00%> (-1.07%) ⬇️
planner/commit.go 77.54% <100.00%> (+1.20%) ⬆️
planner/mapper/mapper.go 87.34% <100.00%> (+0.06%) ⬆️
http/client.go 51.84% <84.62%> (+7.14%) ⬆️
db/schema.go 85.22% <80.56%> (-1.28%) ⬇️
... and 4 more

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdb8ff8...53f3f10. Read the comment docs.

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM. Nice change :)

Function has been preserved as syntax sugar here, as the return type is worth it in this case I think (can only ever return one item, or an error).
Sorry about this one, collection describe was pretty broken, only respecting the get-inactive flag and none of the others, and I didn't realize until quite late. It didn't seem worth going back and re-committing everything with this fix in first.
Misc clean after fixing collection describe, I didn't want to lump this in with the actual fix.
Broken out to own commit to avoid confusing the actual concrete changes made in the next few commits.
Function preserved as syntax sugar as the return type is handy
@AndrewSisley AndrewSisley merged commit 392bd96 into sourcenetwork:develop Feb 20, 2024
31 of 32 checks passed
@AndrewSisley AndrewSisley deleted the 2008-col-schema-filter-opts branch February 20, 2024 18:05
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…rk#2319)

## Relevant issue(s)

Resolves sourcenetwork#2008

## Description

Reworks the GetCollection/SchemaByFoo funcs into single GetCollections
and GetSchemas funcs.
@islamaliev
Copy link
Contributor

bug bash issue #2371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/collections Related to the collections system code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace [Schema]/[Collection]By[Foo] funcs with filterable param
3 participants