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

Update DevDax provider config API #928

Merged
merged 3 commits into from
Nov 28, 2024
Merged

Update DevDax provider config API #928

merged 3 commits into from
Nov 28, 2024

Conversation

vinser52
Copy link
Contributor

@vinser52 vinser52 commented Nov 22, 2024 •
edited
Loading

Description

Change the L0 provider config as we agreed in #844

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly
  • New tests added, especially if they will fail without my changes

@vinser52 vinser52 requested a review from a team as a code owner November 22, 2024 14:43
@vinser52 vinser52 force-pushed the svinogra_devdax_params branch 7 times, most recently from 30726c6 to 636d038 Compare November 22, 2024 20:03
@vinser52 vinser52 added the v0.10.x Should be merged before making stable branch v0.10.x label Nov 22, 2024
@vinser52 vinser52 force-pushed the svinogra_devdax_params branch 2 times, most recently from 338f117 to d6f4fa2 Compare November 25, 2024 08:50
@vinser52 vinser52 requested a review from ldorau November 25, 2024 12:31
ldorau
ldorau requested changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@ldorau ldorau left a comment

Choose a reason for hiding this comment

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

The code coverage of the src/provider/provider_devdax_memory.c file on the main branch is 80.2% while on this PR only 75.6%

@vinser52
Copy link
Contributor Author

The code coverage of the src/provider/provider_devdax_memory.c file on the main branch is 80.2% while on this PR only 75.6%

Can we somehow see which lines are not covered?

@lukaszstolarczuk
Copy link
Contributor

The code coverage of the src/provider/provider_devdax_memory.c file on the main branch is 80.2% while on this PR only 75.6%

Can we somehow see which lines are not covered?

when you enter summary of the actions (here for your PR you can scroll down for artifacts - there's coverage_html_report included (to browse locally on your computer)

ldorau
ldorau approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@ldorau ldorau left a comment

Choose a reason for hiding this comment

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

The code coverage of the src/provider/provider_devdax_memory.c file on the main branch is 80.2% while on this PR 78.7% - still can be better ;-)

@vinser52 vinser52 force-pushed the svinogra_devdax_params branch 2 times, most recently from caa254b to 07bec6a Compare November 27, 2024 11:02
ldorau
ldorau approved these changes Nov 27, 2024
View reviewed changes
Copy link
Contributor

@ldorau ldorau left a comment

Choose a reason for hiding this comment

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

The code coverage of the src/provider/provider_devdax_memory.c file is 82.5% now - great! :-) Thanks!

@vinser52 vinser52 requested a review from lplewa November 27, 2024 23:24
lukaszstolarczuk
lukaszstolarczuk reviewed Nov 28, 2024
View reviewed changes
include/umf/providers/provider_devdax_memory.h Show resolved Hide resolved
test/pools/jemalloc_coarse_devdax.cpp Show resolved Hide resolved
src/provider/provider_devdax_memory.c Outdated Show resolved Hide resolved
test/provider_devdax_memory.cpp Show resolved Hide resolved
test/provider_devdax_memory.cpp Outdated Show resolved Hide resolved
@lukaszstolarczuk lukaszstolarczuk merged commit 38705d6 into oneapi-src:main Nov 28, 2024
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lplewa lplewa lplewa approved these changes

@bratpiorka bratpiorka bratpiorka approved these changes

@ldorau ldorau ldorau approved these changes

@lukaszstolarczuk lukaszstolarczuk Awaiting requested review from lukaszstolarczuk

Assignees
No one assigned
Labels
v0.10.x Should be merged before making stable branch v0.10.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vinser52 @lukaszstolarczuk @ldorau @bratpiorka @lplewa