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

refactor: Merge duplicate input args #3046

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

nasdf
Copy link
Member

@nasdf nasdf commented Sep 20, 2024

Relevant issue(s)

Resolves #3045
Resolves #3042

Description

This PR merges the duplicate input args docID, docIDs, input, and inputs into a single list arg. With GraphQL input coercing the inputs can remain the same for lists as well as single items.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Updated integration tests

Specify the platform(s) on which this was tested:

  • MacOS

@nasdf nasdf added area/query Related to the query component refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Sep 20, 2024
@nasdf nasdf added this to the DefraDB v0.14 milestone Sep 20, 2024
@nasdf nasdf self-assigned this Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.44%. Comparing base (701a7a5) to head (619e438).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3046      +/-   ##
===========================================
- Coverage    79.46%   79.44%   -0.02%     
===========================================
  Files          342      342              
  Lines        26529    26514      -15     
===========================================
- Hits         21080    21062      -18     
- Misses        3933     3938       +5     
+ Partials      1516     1514       -2     
Flag Coverage Δ
all-tests 79.44% <100.00%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
client/request/mutation.go 100.00% <ø> (ø)
internal/planner/create.go 81.16% <100.00%> (-0.53%) ⬇️
internal/planner/delete.go 78.46% <100.00%> (ø)
internal/planner/group.go 87.18% <100.00%> (ø)
internal/planner/mapper/mapper.go 88.82% <ø> (-0.01%) ⬇️
internal/planner/select.go 85.11% <100.00%> (ø)
internal/planner/update.go 78.75% <100.00%> (+0.27%) ⬆️
internal/request/graphql/parser/mutation.go 85.92% <100.00%> (-0.93%) ⬇️
internal/request/graphql/parser/query.go 86.02% <ø> (-0.22%) ⬇️
internal/request/graphql/schema/generate.go 87.97% <100.00%> (-0.08%) ⬇️

... 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 701a7a5...619e438. 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. I like that you kept the singular params. Thanks for this.

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

LGTM

@nasdf nasdf merged commit 0b2a90d into sourcenetwork:develop Sep 23, 2024
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/query Related to the query component refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge docID and docIDs input Merge mutation input and inputs argument
3 participants