Skip to content

Conversation

joyceerhl
Copy link

  • 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.

@joyceerhl joyceerhl added the no-changelog No news entry required label Oct 21, 2020
@codecov-io
Copy link

codecov-io commented Oct 21, 2020

Codecov Report

Merging #14460 into release-2020.10 will decrease coverage by 0.02%.
The diff coverage is 54.16%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           release-2020.10   #14460      +/-   ##
===================================================
- Coverage            59.19%   59.17%   -0.03%     
===================================================
  Files                  720      721       +1     
  Lines                40211    40228      +17     
  Branches              5828     5828              
===================================================
+ Hits                 23802    23803       +1     
- Misses               15147    15160      +13     
- Partials              1262     1265       +3     
Impacted Files Coverage Δ
...ience/interactive-common/interactiveWindowTypes.ts 100.00% <ø> (ø)
src/client/telemetry/index.ts 80.41% <ø> (ø)
src/client/datascience/commands/commandRegistry.ts 34.76% <25.00%> (+0.23%) ⬆️
...ata-viewing/debuggerDataViewerExperimentEnabler.ts 42.85% <42.85%> (ø)
src/client/common/serviceRegistry.ts 100.00% <100.00%> (ø)
src/client/datascience/constants.ts 99.78% <100.00%> (-0.01%) ⬇️
src/client/telemetry/constants.ts 100.00% <100.00%> (ø)
...very/locators/services/virtualenvwrapperLocator.ts 76.92% <0.00%> (-15.39%) ⬇️
src/client/common/utils/platform.ts 60.00% <0.00%> (-8.00%) ⬇️
.../pythonEnvironments/common/externalDependencies.ts 64.10% <0.00%> (-2.57%) ⬇️
... 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 2971ac5...c081403. Read the comment docs.

Copy link
Member

@IanMatthewHuff IanMatthewHuff left a comment

Choose a reason for hiding this comment

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

Is this for the October release? Because the myNoDS PR is going to trash these changes when it hits.

@joyceerhl
Copy link
Author

@IanMatthewHuff yeah this is for October. Maybe this should be going straight into the release branch then, since I'm going to rework the integration for November?

@IanMatthewHuff
Copy link
Member

@IanMatthewHuff yeah this is for October. Maybe this should be going straight into the release branch then, since I'm going to rework the integration for November?

I think that might be the smart approach to take, good thought on going directly to release. Otherwise this will just cause merge issues for the myNoDS change.

@joyceerhl joyceerhl changed the base branch from main to release-2020.10 October 21, 2020 18:19
@joyceerhl joyceerhl changed the base branch from release-2020.10 to main October 21, 2020 18:19
@joyceerhl joyceerhl changed the base branch from main to release-2020.10 October 21, 2020 18:30
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@joyceerhl
Copy link
Author

@DonJayamanne I believe I've addressed your comment on this PR!

@DonJayamanne DonJayamanne merged commit d63031c into release-2020.10 Oct 21, 2020
@DonJayamanne DonJayamanne deleted the exp branch October 21, 2020 21:27
@karthiknadig
Copy link
Member

@joyceerhl we need the Compiler fix in master. Insiders is currently not getting pushed out because of it.

@joyceerhl
Copy link
Author

@karthiknadig Will port it over, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants