Skip to content

Conversation

rchiodo
Copy link

@rchiodo rchiodo commented Nov 20, 2019

For #8677

@codecov-io
Copy link

codecov-io commented Nov 20, 2019

Codecov Report

Merging #8699 into master will decrease coverage by 0.31%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8699      +/-   ##
==========================================
- Coverage   59.17%   58.85%   -0.32%     
==========================================
  Files         521      521              
  Lines       23981    23984       +3     
  Branches     3874     3869       -5     
==========================================
- Hits        14190    14116      -74     
- Misses       8874     8951      +77     
  Partials      917      917
Impacted Files Coverage Δ
...ient/datascience/interactive-ipynb/nativeEditor.ts 61.72% <100%> (ø) ⬆️
src/client/datascience/jupyter/jupyterImporter.ts 18.98% <11.76%> (+0.56%) ⬆️
src/client/testing/serviceRegistry.ts 48.64% <0%> (-46.85%) ⬇️
src/client/testing/codeLenses/main.ts 44.44% <0%> (-33.34%) ⬇️
...t/datascience/jupyter/jupyterDataRateLimitError.ts 66.66% <0%> (-33.34%) ⬇️
src/client/formatters/serviceRegistry.ts 75% <0%> (-25%) ⬇️
...ience/jupyter/jupyterDebuggerRemoteNotSupported.ts 75% <0%> (-25%) ⬇️
...cience/jupyter/jupyterDebuggerNotInstalledError.ts 60% <0%> (-20%) ⬇️
src/client/common/utils/icons.ts 83.33% <0%> (-16.67%) ⬇️
src/client/api.ts 78.57% <0%> (-14.29%) ⬇️
... and 4 more

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 d41e10d...81a7ccd. Read the comment docs.

Copy link

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

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

Curious why we're suppressing the error. I'd like to know about the error.

} else {
return undefined;
}
} catch (e) {

Choose a reason for hiding this comment

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

Why not throw the error?

Copy link
Author

Choose a reason for hiding this comment

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

IMHO, in this situation, it's better to have the python script be output instead of breaking the user's conversion. Plus it makes the test pass.

@rchiodo rchiodo merged commit 1782b75 into master Nov 20, 2019
@rchiodo rchiodo deleted the rchiodo/fix_convert branch November 20, 2019 23:25
rchiodo added a commit that referenced this pull request Nov 21, 2019
* Fix converting from a python script

* Fix linter error
karthiknadig added a commit that referenced this pull request Nov 27, 2019
* Ensure we run PR, CI pipelines on release branches (#8471) (#8474)

* Port blue icon and altair background fixes into release (#8470)

* fix blue color on export icon (#8449)

* default non-text mimetypes to white background (#8452)

* Fix cells being erased when saving and then changing focus to another… (#8482) (#8485)

* Fix cells being erased when saving and then changing focus to another cell

* Make sure to use the source even if it's empty

* Fix timeout to be what it was before. 10 seconds isn't long enough (#8504)

* Ensure wheels experiment control and experiment groups uses right wheel (#8461) (#8546)

* Ensure wheels control group uses no wheel ptvsd
* Improve condition readability in jupyter debugger

* Adjust debug adapter experiment before release  (#8540) (#8553)

* Release branch: updating change log and package json (#8570)

* Change version numbers

* Update package lock json

* Update changelog

* Fix for 7999 - CTRL+Z should not undo at the top level (#8571)

* Fix arrowing up and down to save code too (#8574)

* Prompt selecting a file to save once (#8590)

* Prompt selecting a file to save once
* Better algorithm

* After pasting code, arrow keys don't navigate in a cell. (#8587)

* Don't look for a jupyter interpreter when creating blank notebook (#8596)

* Creating a new blank notebook should not require a search for jupyter.

* Fix bug caused by other fix.

* Update change log

* Update release date in change log (#8604)

* Fix converting from a python script (#8699)

* Fix converting from a python script

* Fix linter error

* Fix problem with $$ not being put around the correct items in a LaTeX equation (#8707)

* First try

* Add news entry and fix multiples

* Add a bunch of comments

* Modify changelog and version number

* Fixes to starting Jupyter in a Docker container. (#8715)

* Ensure we generate the right args when running in docker
* Fixes to starting `Jupyter` in a `Docker` container.

* Ensure arguments are generated correctly for `getRemoteLauncherCommand` when in debugger experiment (#8712)

* Return new debugger arguments when in experiment

* Add news item

* Updates to change log

* Revert tests due to incompatibilities
@lock lock bot locked as resolved and limited conversation to collaborators Nov 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants