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: Rename all Query terminology to Request #497

Closed

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Jun 2, 2022

RELEVANT ISSUE(S)

Resolves #455

DESCRIPTION

Change the codebase Query terminology to use the word Request to address the top level Query Request and to use the word query to address the Read-only Operation to avoid confusion.

HOW HAS THIS BEEN TESTED?

Locally and CI.

CHECKLIST:

  • I have commented the code, particularly in hard-to-understand areas.
  • I have made sure that the PR title adheres to the conventional commit style (subset of the ones we use can be found under: tools/configs/chglog/config.yml

ENVIRONMENT / OS THIS WAS TESTED ON?

Please specify which of the following was this tested on (remove or add your own):

  • Arch Linux

@shahzadlone shahzadlone added code quality Related to improving code quality action/no-benchmark Skips the action that runs the benchmark. labels Jun 2, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3 milestone Jun 2, 2022
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #497 (f5f45af) into develop (45978e0) will increase coverage by 0.01%.
The diff coverage is 63.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #497      +/-   ##
===========================================
+ Coverage    65.09%   65.11%   +0.01%     
===========================================
  Files           87       88       +1     
  Lines        10242    10242              
===========================================
+ Hits          6667     6669       +2     
+ Misses        2927     2925       -2     
  Partials       648      648              
Impacted Files Coverage Δ
api/http/handlerfuncs.go 0.81% <0.00%> (ø)
client/descriptions.go 76.66% <ø> (ø)
db/fetcher/dag.go 57.31% <ø> (ø)
db/fetcher/versioned.go 49.47% <ø> (ø)
request/graphql/parser/commit.go 59.61% <ø> (ø)
request/graphql/parser/filter.go 69.60% <0.00%> (ø)
request/graphql/parser/mutation.go 61.60% <ø> (ø)
request/graphql/parser/query.go 82.39% <ø> (ø)
request/graphql/planner/arbitrary_join.go 77.58% <ø> (ø)
request/graphql/planner/average.go 73.46% <ø> (ø)
... and 35 more

@shahzadlone shahzadlone marked this pull request as draft June 2, 2022 20:07
@shahzadlone shahzadlone self-assigned this Jun 3, 2022
@AndrewSisley
Copy link
Contributor

ah.... I didnt realize you'd end up moving the planner/parser packages.... How long did you spend on this? Is it quick to do? Am quite nervous about missing changes in develop in the doc restructure branch if you are moving these files (doc restructure is close to proper PR status now)

@shahzadlone shahzadlone force-pushed the lone/refactor/use-of-request-and-query-terminology branch from 76e6cbd to f5f45af Compare June 7, 2022 15:56
@shahzadlone
Copy link
Member Author

shahzadlone commented Jun 7, 2022

ah.... I didnt realize you'd end up moving the planner/parser packages.... How long did you spend on this? Is it quick to do? Am quite nervous about missing changes in develop in the doc restructure branch if you are moving these files (doc restructure is close to proper PR status now)

This is why I mentioned in the last meeting that we will have notable conflicts with your stuff. Nothing to worry this is all fairly easy to patch on top of your work once it is in, it should be straight forward from my end as this didn't take too much time (most of the work was done with vim macros and tools).

Will say this definitely should happen after your stuff is in (or even later if there are any other major conflicting PR changes).

Additionally adding the Do not merge label.

@shahzadlone shahzadlone marked this pull request as ready for review June 7, 2022 16:01
@AndrewSisley
Copy link
Contributor

This is why I mentioned in the last meeting that we will have notable conflicts with your stuff.

Ah sorry, I guess I'd either forgotten that or didn't realize the impact of you moving files (and thought it would just be simple/normal conflicts)

Will say this definitely should happen after your stuff is in (or even later if there are any other major conflicting PR changes).

Cheers! Will review properly after the standup then

@@ -152,7 +153,7 @@ func (hf *HeadFetcher) Close() error {
// List returns the list of current heads plus the max height.
// @todo Document Heads.List function
func (hh *heads) List() ([]cid.Cid, uint64, error) {
q := query.Query{
q := dsRequest.Query{
Copy link
Member Author

Choose a reason for hiding this comment

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

undo

@shahzadlone shahzadlone marked this pull request as draft June 19, 2022 08:30
@jsimnz jsimnz modified the milestones: DefraDB v0.3, DefraDB v0.4 Aug 4, 2022
@jsimnz jsimnz modified the milestones: DefraDB v0.4, DefraDB v0.5 Jan 20, 2023
@shahzadlone
Copy link
Member Author

Closing for: #1054

@shahzadlone shahzadlone deleted the lone/refactor/use-of-request-and-query-terminology branch April 13, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. code quality Related to improving code quality DO NOT MERGE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactoring All Query Request Terminology to be change to Request
3 participants