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.

Sign up for GitHub

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

Jump to bottom

feat: Update countNode explanation to be consistent. #600

Merged
merged 4 commits into from
Jul 9, 2022
Merged

feat: Update countNode explanation to be consistent. #600

merged 4 commits into from
Jul 9, 2022

Conversation

shahzadlone
Copy link
Member

Relevant issue(s)

Resolves #599

Description

Rename sourceProperty to fieldName to be consistent with sumNode and more in harmony with the post document restructure changes.

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?

CI and make test

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

  • Arch Linux

@shahzadlone shahzadlone added area/query Related to the query component refactor This issue specific to or requires *notable* refactoring of existing codebases and components action/no-benchmark Skips the action that runs the benchmark. labels Jul 7, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3 milestone Jul 7, 2022
@shahzadlone shahzadlone requested a review from a team July 7, 2022 13:17
@shahzadlone shahzadlone self-assigned this Jul 7, 2022
AndrewSisley
AndrewSisley approved these changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM

query/graphql/planner/count.go Outdated Show resolved Hide resolved
@shahzadlone shahzadlone force-pushed the lone/feat/consistent-count-node-explain-attributes branch from 5218af2 to a0f0919 Compare July 7, 2022 14:22
fredcarle
fredcarle approved these changes Jul 7, 2022
View reviewed changes
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.

🤘

@shahzadlone shahzadlone force-pushed the lone/feat/consistent-count-node-explain-attributes branch from a0f0919 to 67d26ee Compare July 9, 2022 13:30
@shahzadlone shahzadlone changed the title feat: Update countNode attributes to be consistent with sumNode. feat: Update countNode explanation to be consistent. Jul 9, 2022
@shahzadlone shahzadlone merged commit 16084c8 into develop Jul 9, 2022
@shahzadlone shahzadlone deleted the lone/feat/consistent-count-node-explain-attributes branch July 9, 2022 13:34
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@shahzadlone
feat: Update countNode explanation to be consistent. (sourcenetwork…
7e58d4a 
…#600)

Resolves sourcenetwork#599 

- Description: Rename `sourceProperty` to `fieldName` to be consistent with `sumNode` and more in harmony with the post document restructure changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrewSisley AndrewSisley AndrewSisley approved these changes

@fredcarle fredcarle fredcarle approved these changes

Assignees

@shahzadlone shahzadlone

Labels
action/no-benchmark Skips the action that runs the benchmark. area/query Related to the query component refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Milestone
DefraDB v0.3
Development

Successfully merging this pull request may close these issues.

Make countNode explain more consistent with other nodes (sumNode)
3 participants
@shahzadlone @fredcarle @AndrewSisley