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

make 'cwd' param for TerminalManager absolute #749

Merged
merged 1 commit into from Mar 23, 2022
Merged

make 'cwd' param for TerminalManager absolute #749

merged 1 commit into from Mar 23, 2022

Conversation

rccern
Copy link
Contributor

@rccern rccern commented Mar 23, 2022

needed for jupyterlab/jupyterlab#12250 as discussed with @fcollonval
I was also thinking about a test. If here https://github.com/jupyter-server/jupyter_server/blob/main/tests/test_terminal.py#L83 I put str(os.path.basename(terminal_path)) then the test 'hangs' (as expected, but it's not ideal in the test). Otherwise we can check for str(terminal_path) instead of os.path.basename here https://github.com/jupyter-server/jupyter_server/blob/main/tests/test_terminal.py#L136

@welcome
Copy link

welcome bot commented Mar 23, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@codecov-commenter
Copy link

Codecov Report

Merging #749 (0964b23) into main (3864399) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

❗ Current head 0964b23 differs from pull request most recent head b92362d. Consider uploading reports for the commit b92362d to get more accurate results

@@            Coverage Diff             @@
##             main     #749      +/-   ##
==========================================
- Coverage   70.51%   70.48%   -0.03%     
==========================================
  Files          62       62              
  Lines        7615     7621       +6     
  Branches     1216     1218       +2     
==========================================
+ Hits         5370     5372       +2     
- Misses       1877     1880       +3     
- Partials      368      369       +1     
Impacted Files Coverage Δ
jupyter_server/terminal/api_handlers.py 89.47% <33.33%> (-10.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3864399...b92362d. Read the comment docs.

@blink1073 blink1073 added the bug label Mar 23, 2022
Copy link
Collaborator

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 merged commit 65b06aa into jupyter-server:main Mar 23, 2022
@welcome
Copy link

welcome bot commented Mar 23, 2022

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants