Skip to content

Conversation

@janosh
Copy link

@janosh janosh commented Sep 5, 2019

For #6842 (following #7163) (needs further discussion)

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Appropriate comments and documentation strings in the code
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • Test plan is updated as appropriate
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • The wiki is updated with any design decisions/details.

python.dataScience.runStartupCommands is only triggered when launching a fresh interactive window. It should, in my opinion, also trigger when restarting the kernel. Users will expect that restarting the kernel results in a clean but identical state to when they first launched the interactive window. Currently, this is not the case. If maintainers agree, I'm happy to add a fix to this PR.

@codecov-io
Copy link

codecov-io commented Sep 5, 2019

Codecov Report

Merging #7216 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #7216      +/-   ##
=========================================
- Coverage   58.55%   58.5%   -0.06%     
=========================================
  Files         486     486              
  Lines       21460   21460              
  Branches     3463    3463              
=========================================
- Hits        12566   12555      -11     
- Misses       8116    8129      +13     
+ Partials      778     776       -2
Impacted Files Coverage Δ
src/client/common/logger.ts 68.55% <0%> (-6.92%) ⬇️

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 a70b4f5...0d560d2. Read the comment docs.

@rchiodo rchiodo added the no-changelog No news entry required label Sep 6, 2019
@rchiodo rchiodo merged commit 19546f4 into microsoft:master Sep 6, 2019
@rchiodo
Copy link

rchiodo commented Sep 6, 2019

@janosh, I believe we're about to change this such that it will rerun the commands on a kernel restart as well. That's in this PR here:
#7222

@rchiodo
Copy link

rchiodo commented Sep 6, 2019

Yeah it's on line 200 or so of the new jupyterNotebook.ts file. notebook.initialize gets called every time we restart.

@janosh
Copy link
Author

janosh commented Sep 6, 2019

Very cool! #7222 is a hell of a PR! You guys are on fire. 😄

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants