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

[master < T1080] overload << operator for mgp::Type #1080

Merged
merged 6 commits into from Jul 25, 2023

Conversation

mpintaric55334
Copy link
Contributor

@mpintaric55334 mpintaric55334 commented Jul 19, 2023

During writing collections query module, the need to be able to print the mgp::Type was found, so it was solved with this PR.

[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 print the type of mgp::Type() using operator << . For example, if you have a mgp::List() list, cout<< list <<endl will output "list".

  • Tag someone from docs team in the comments

@mpintaric55334 mpintaric55334 self-assigned this Jul 19, 2023
@gitbuda gitbuda changed the title Type string for mgp matija pintaric Add TypeString method to C++ query modules API Jul 19, 2023
@mpintaric55334 mpintaric55334 force-pushed the TypeString_for_mgp-matija-pintaric branch from 3956715 to 5b821e7 Compare July 21, 2023 13:41
include/mgp.hpp Outdated Show resolved Hide resolved
include/mgp.hpp Outdated Show resolved Hide resolved
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.

I would change that we return "" instead of "type: ", otherwise looks good

@antoniofilipovic
Copy link
Collaborator

@mpintaric55334 there is one other thing I haven't thought about until now. We can support instead of TypeString, operator<< which serves for output: https://learn.microsoft.com/en-us/cpp/standard-library/overloading-the-output-operator-for-your-own-classes?view=msvc-170. This is one example, what do you think?

@mpintaric55334 mpintaric55334 added the Ready for review PR is ready for review label Jul 24, 2023
@mpintaric55334 mpintaric55334 changed the title Add TypeString method to C++ query modules API [master < T1080] overload << operator for mgp::Type Jul 24, 2023
@mpintaric55334
Copy link
Contributor Author

@vpavicic will update the docs when I get the green light from Antonio.

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 Ship it PR ready to be merged and removed Ready for review PR is ready for review labels Jul 25, 2023
@antoniofilipovic antoniofilipovic merged commit ab4d1ef into master Jul 25, 2023
6 checks passed
@antoniofilipovic antoniofilipovic deleted the TypeString_for_mgp-matija-pintaric branch July 25, 2023 11:30
@vpavicic
Copy link
Contributor

Is this also C++ ?

@mpintaric55334
Copy link
Contributor Author

Yes, it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ship it PR ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants