Skip to content

Conversation

@antoniofilipovic
Copy link
Contributor

@antoniofilipovic antoniofilipovic commented Jan 9, 2023

This PR fixes few bugs:

  • there was a problem with getting Properties - since std::string_view was used as a key to store Properties in std::map, due to the nature of how properties are retrieved from mgp_properties_get_next in combination with std::string_view, the new property would overwrite the old one and the user wouldn't get the correct state of properties (some would miss, std::map would be filled with same property)
  • there was a missing set_property function in both mgp.hpp and _mgp.hpp

[master < Task] PR

  • Check, and update documentation if necessary -> yes
  • Update changelog -> still no
  • Provide the full content or a guide for the final git message: Fix bug on (vertex|edge) properties in C++ API

Copy link
Contributor

@antepusic antepusic left a comment

Choose a reason for hiding this comment

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

Good work! Would it make sense to replace string_view with string elsewhere in the API as well?

@antoniofilipovic
Copy link
Contributor Author

Good work! Would it make sense to replace string_view with string elsewhere in the API as well?

Since there are no problems, I would leave it as is.

Copy link
Contributor

@antaljanosbenjamin antaljanosbenjamin left a comment

Choose a reason for hiding this comment

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

Please see my comment about StealType.

@gitbuda gitbuda added v2.5.2 bug bug and removed v2.5.1 labels Jan 19, 2023
Copy link
Contributor

@antepusic antepusic 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 merged commit 1cd1da8 into master Jan 23, 2023
@antoniofilipovic antoniofilipovic deleted the T1220-MG-properties-c++-api-bug branch January 23, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants