-
Notifications
You must be signed in to change notification settings - Fork 4
Add generic interactive shell #189
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
Conversation
I will request an initial review to decide on steps forward (i.e., reverting wip commits and keeping generic, or continuing on transport context). The failed test is due to asserting a specific message is printing out, where the terminal is now injecting it's messages (e.g., 'In [1]:). Should we disable the terminal with some test ENV VAR instead of working around it in these sorts of tests? |
I've removed implementations of transport context for pva; instead I made this #190 for CA. This PR is now only the generic implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #189 +/- ##
==========================================
- Coverage 92.25% 92.20% -0.06%
==========================================
Files 40 40
Lines 2040 2065 +25
==========================================
+ Hits 1882 1904 +22
- Misses 158 161 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I've commented out |
I think I am OK with leaving that run method with missing lines unless you can come up with a neat way to test that |
Hit my head against it for a little while, and can't figure out a "neat" way to do it; I can open up a small issue for me to look at after, but for now we should probably merge this in (if you are happy with my other test)? |
fixes #140
This PR adds an interactive shell, based on this implementation, and adds transport contexts. The
wip
commits are attempts to adddbl
dbpf
anddbgf
functionality toPVA
as an example, but maybe only the first commit is required for this PR, and further PRs can add transport specific context.