Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

[inline_string] Differentiate inline_string on PMEM and DRAM#1166

Merged
lukaszstolarczuk merged 2 commits intopmem:masterfrom
KFilipek:task-inline_string_change
Jul 20, 2021
Merged

[inline_string] Differentiate inline_string on PMEM and DRAM#1166
lukaszstolarczuk merged 2 commits intopmem:masterfrom
KFilipek:task-inline_string_change

Conversation

@KFilipek
Copy link
Copy Markdown
Contributor

@KFilipek KFilipek commented Jul 19, 2021 •
edited by lukaszstolarczuk
Loading

This change is Reviewable

@KFilipek KFilipek requested a review from igchor July 20, 2021 11:34
@KFilipek KFilipek self-assigned this Jul 20, 2021
@KFilipek KFilipek added the Type: Feature New feature or request label Jul 20, 2021
@KFilipek KFilipek marked this pull request as ready for review July 20, 2021 11:34
igchor
igchor approved these changes Jul 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @KFilipek)

KFilipek added 2 commits July 20, 2021 14:08
@KFilipek
[inline_string] Differentiate inline_string on PMEM and DRAM
bf3438a
@KFilipek
inline_string: refactor compare() to take string_view
1fc1121 
instead of basic_inline_string so that there is no implicit
copy of inline_string when string_view is passed.

Authored by: Igor Chorazewicz
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 20, 2021 •
edited
Loading

Codecov Report

Merging #1166 (1fc1121) into master (a05e6c1) will increase coverage by 0.00%.
The diff coverage is 96.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1166   +/-   ##
=======================================
  Coverage   94.07%   94.07%           
=======================================
  Files          52       52           
  Lines        5163     5135   -28     
=======================================
- Hits         4857     4831   -26     
+ Misses        306      304    -2
Flag Coverage Δ
tests_clang_debug_cpp17 93.62% <96.00%> (-0.01%) ⬇️
tests_gcc_debug 90.70% <96.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...nclude/libpmemobj++/experimental/inline_string.hpp 98.46% <96.00%> (+0.68%) ⬆️
...ude/libpmemobj++/container/concurrent_hash_map.hpp 94.04% <0.00%> (-0.16%) ⬇️
include/libpmemobj++/experimental/radix_tree.hpp 98.46% <0.00%> (+0.14%) ⬆️
include/libpmemobj++/detail/life.hpp 97.43% <0.00%> (+0.21%) ⬆️
include/libpmemobj++/container/array.hpp 98.07% <0.00%> (+0.42%) ⬆️
include/libpmemobj++/persistent_ptr.hpp 97.53% <0.00%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a05e6c1...1fc1121. Read the comment docs.

@KFilipek KFilipek requested a review from karczex July 20, 2021 13:53
karczex
karczex reviewed Jul 20, 2021
View reviewed changes
Copy link
Copy Markdown

@karczex karczex left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @KFilipek)

lukaszstolarczuk
lukaszstolarczuk reviewed Jul 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

👍

Reviewed 1 of 2 files at r2, 1 of 2 files at r3, 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @KFilipek)

@lukaszstolarczuk lukaszstolarczuk merged commit 5c9d3b8 into pmem:master Jul 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@karczex karczex karczex left review comments

@lukaszstolarczuk lukaszstolarczuk lukaszstolarczuk left review comments

@igchor igchor igchor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@KFilipek KFilipek

Labels

Type: Feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@KFilipek @karczex @igchor @lukaszstolarczuk