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

[master < T1090] Add overload for operator< in mgp::Value #1090

Merged
merged 10 commits into from Jul 28, 2023
Merged

[master < T1090] Add overload for operator< in mgp::Value #1090

merged 10 commits into from Jul 28, 2023

Conversation

mpintaric55334
Copy link
Contributor

@mpintaric55334 mpintaric55334 commented Jul 21, 2023 •
edited

Before, when writing C++ query moduels, directly comparing two mgp::Value variables was not possible. With this PR, it is.

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here
    You are now able to compare two mgp::Value variables with < operator.

  • Tag someone from docs team in the comments

@mpintaric55334 mpintaric55334 self-assigned this Jul 21, 2023
antoniofilipovic
antoniofilipovic reviewed Jul 21, 2023
View reviewed changes
include/mgp.hpp Outdated Show resolved Hide resolved
antoniofilipovic
antoniofilipovic requested changes Jul 21, 2023
View reviewed changes
Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

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

Few stuff and it should be good to ship

@mpintaric55334 mpintaric55334 added the Ready for review PR is ready for review label Jul 24, 2023
antoniofilipovic
antoniofilipovic reviewed Jul 24, 2023
View reviewed changes
include/mgp.hpp Outdated Show resolved Hide resolved
antoniofilipovic
antoniofilipovic requested changes Jul 24, 2023
View reviewed changes
Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

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

Few comments left and that should be it

@antoniofilipovic antoniofilipovic added Needs change PR needs change and removed Ready for review PR is ready for review labels Jul 24, 2023
@mpintaric55334 mpintaric55334 changed the title Add overload for less operator mgp value [master < T1090] Add overload for less operator in mgp::value Jul 24, 2023
@mpintaric55334 mpintaric55334 force-pushed the add-overload-for-less-operator-mgp-value branch from 4693be8 to f66132f Compare July 24, 2023 13:02
@mpintaric55334 mpintaric55334 added Ready for review PR is ready for review and removed Needs change PR needs change labels Jul 24, 2023
@mpintaric55334
Copy link
Contributor Author

@vpavicic I will start writing docs after green light from Antonio.

antoniofilipovic
antoniofilipovic approved these changes Jul 25, 2023
View reviewed changes
Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

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

Looks good to go

@antoniofilipovic antoniofilipovic added the Ship it PR ready to be merged label Jul 25, 2023
@antoniofilipovic antoniofilipovic removed the Ready for review PR is ready for review label Jul 25, 2023
@antoniofilipovic antoniofilipovic changed the title [master < T1090] Add overload for less operator in mgp::value [master < T1090] Add overload for operator< in mgp::Value Jul 25, 2023
@antoniofilipovic antoniofilipovic merged commit 76ca019 into master Jul 28, 2023
6 checks passed
@antoniofilipovic antoniofilipovic deleted the add-overload-for-less-operator-mgp-value branch July 28, 2023 09:35
@mpintaric55334 mpintaric55334 mentioned this pull request Aug 1, 2023
Merged
6 tasks
as51340 pushed a commit that referenced this pull request Nov 10, 2023
@mpintaric55334 @as51340
Add overload of operator< for mgp::Value (#1090)
b8f86d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoniofilipovic antoniofilipovic antoniofilipovic approved these changes

Assignees

@mpintaric55334 mpintaric55334

Labels
Ship it PR ready to be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mpintaric55334 @antoniofilipovic