compiler-research / xeus-cpp

Jupyter kernel for the C++ programming language
BSD 3-Clause "New" or "Revised" License
17 stars 24 forks source link

Upgrade to xeus 5 #127

Closed anutosh491 closed 3 months ago

anutosh491 commented 3 months ago

Fixes #108 Closes #110

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

mcbarton commented 3 months ago

@anutosh491 maybe link this to the issues about removing the xtl dependency and update the readme about depenencies on the main

mcbarton commented 3 months ago

@anutosh491 The failing test is the one which loads the image as it makes use of xtl when it loads the xtl header

include "xtl/xbase64.hpp"

You'll also need to change this in the example notebook.

anutosh491 commented 3 months ago

as it makes use of xtl when it loads the xtl header

Ahh I overlooked this. Thanks for the debug :)

mcbarton commented 3 months ago

@anutosh491 will this also fix https://github.com/compiler-research/xeus-cpp/issues/54 since its related to xtl?

anutosh491 commented 3 months ago

Hmm, let me think through it. I think it might just solve that !

anutosh491 commented 3 months ago

@anutosh491 will this also fix #54 since its related to xtl?

Well the 2 problems have some overlap but are a bit different. I think I have a way to tackle the issue, will solve it through a subsequent PR.

mcbarton commented 3 months ago

@anutosh491 xeus_REQUIRED_VERSION needs to be version 5.1.0 now

anutosh491 commented 3 months ago

@anutosh491 xeus_REQUIRED_VERSION needs to be version 5.1.0 now

Yeah I think I would use xeus >= 5.0.0 in the environment. Just was debugging if we were able to fetch 5.1.0. So required version could be 5.0.0

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

anutosh491 commented 3 months ago

cc @alexander-penev @vgvassilev @mcbarton This is ready now !

anutosh491 commented 3 months ago

Ahh okay 1 thing. I'll address the readme too :)

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 91.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 79.41%. Comparing base (60fb73d) to head (df310a7). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127/graphs/tree.svg?width=650&height=150&src=pr&token=9KM610P5A4&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research)](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) ```diff @@ Coverage Diff @@ ## main #127 +/- ## ========================================== + Coverage 79.31% 79.41% +0.10% ========================================== Files 17 17 Lines 609 612 +3 Branches 59 59 ========================================== + Hits 483 486 +3 Misses 126 126 ``` | [Files](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) | Coverage Δ | | |---|---|---| | [src/xinspect.hpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fxinspect.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL3hpbnNwZWN0LmhwcA==) | `69.49% <ø> (ø)` | | | [src/xinterpreter.cpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fxinterpreter.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL3hpbnRlcnByZXRlci5jcHA=) | `88.67% <100.00%> (+0.07%)` | :arrow_up: | | [src/main.cpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fmain.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL21haW4uY3Bw) | `42.85% <66.66%> (+2.11%)` | :arrow_up: | | [Files](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research) | Coverage Δ | | |---|---|---| | [src/xinspect.hpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fxinspect.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL3hpbnNwZWN0LmhwcA==) | `69.49% <ø> (ø)` | | | [src/xinterpreter.cpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fxinterpreter.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL3hpbnRlcnByZXRlci5jcHA=) | `88.67% <100.00%> (+0.07%)` | :arrow_up: | | [src/main.cpp](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/127?src=pr&el=tree&filepath=src%2Fmain.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=compiler-research#diff-c3JjL21haW4uY3Bw) | `42.85% <66.66%> (+2.11%)` | :arrow_up: |
github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

anutosh491 commented 3 months ago

cc @vgvassilev ready !