compiler-research / xeus-cpp

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

Remove resource and include directories from kernel json #115

Open mcbarton opened 4 months ago

mcbarton commented 4 months ago

When creating the interpreter we already detect and add the arguments for the resource and include directories using CppInterOp here https://github.com/mcbarton/xeus-cpp/blob/b58b15b83b27fe2e73c00c469346eea65301464d/src/xinterpreter.cpp#L37C1-L56C2 . This makes their addition in the kernel json file redundant.

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 70.09%. Comparing base (6692a45) to head (b58b15b).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/compiler-research/xeus-cpp/pull/115/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/115?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 #115 +/- ## ======================================= Coverage 70.09% 70.09% ======================================= Files 17 17 Lines 602 602 Branches 59 59 ======================================= Hits 422 422 Misses 180 180 ```
mcbarton commented 4 months ago

@anutosh491 @vgvassilev This PR is ready for review.

vgvassilev commented 4 months ago

When creating the interpreter we already detect and add the arguments for the resource and include directories using CppInterOp here https://github.com/mcbarton/xeus-cpp/blob/b58b15b83b27fe2e73c00c469346eea65301464d/src/xinterpreter.cpp#L37C1-L56C2

Unfortunately that works on some platforms depending if we have installed compiler and we can find the compiler...

The resource-dir is particularly tricky to find and in CppInterOp we need to figure out how to make it part of the library. Only then we can drop it and be confident that we are not shooting our feet...

mcbarton commented 4 months ago

When creating the interpreter we already detect and add the arguments for the resource and include directories using CppInterOp here https://github.com/mcbarton/xeus-cpp/blob/b58b15b83b27fe2e73c00c469346eea65301464d/src/xinterpreter.cpp#L37C1-L56C2

Unfortunately that works on some platforms depending if we have installed compiler and we can find the compiler...

The resource-dir is particularly tricky to find and in CppInterOp we need to figure out how to make it part of the library. Only then we can drop it and be confident that we are not shooting our feet...

I'll convert this PR to a draft for now then, until that time comes where we are confident we can drop it.