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.

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

Added demo for inspect request in xeus-cpp-lite-demo.ipynb #260

Closed
wants to merge 3 commits into from

Conversation

kr-2003
Copy link
Contributor

@kr-2003 kr-2003 commented Feb 25, 2025

Description

Since, #229(Inspect request fails on xeus-cpp-lite) is resolved by #258, I have added the demo for inspect request in /notebook/xeus-cpp-lite-demo.ipynb.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.72%. Comparing base (f054951) to head (7487f0a).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #260   +/-   ##
=======================================
  Coverage   80.72%   80.72%           
=======================================
  Files          19       19           
  Lines         970      970           
  Branches       93       93           
=======================================
  Hits          783      783           
  Misses        187      187           

Copy link
Collaborator

@anutosh491 anutosh491 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. Shall Merge after #258 goes in !

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

That needs to become a part of the previous PR, not a separate one.

@anutosh491
Copy link
Collaborator

That needs to become a part of the previous PR, not a separate one.

Sorry It was me who asked him to make 2 separate PRs as I think they are discrete changes and not really dependent on each other. What I wanted actually is 1 commit for the change and 1 commit for the notebook update.

@kr-2003 could you move these changes on your previous PR. We can then move that in

@vgvassilev
Copy link
Contributor

That needs to become a part of the previous PR, not a separate one.

Sorry It was me who asked him to make 2 separate PRs as I think they are discrete changes and not really dependent on each other. What I wanted actually is 1 commit for the change and 1 commit for the notebook update.

@kr-2003 could you move these changes on your previous PR. We can then move that in

Changes and the tests should go in the same commit to make sure we have a clear history describing the change and what it was intended to address/fix.

@anutosh491
Copy link
Collaborator

Perfect, yes !
Let's do that

@kr-2003
Copy link
Contributor Author

kr-2003 commented Feb 25, 2025

Perfect, yes ! Let's do that

Done 👍

@kr-2003 kr-2003 requested a review from vgvassilev February 25, 2025 12:55
@kr-2003 kr-2003 closed this Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants