Skip to content

Ensure we have a base type for all action payloads #9840

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.

Sign up for GitHub

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

Jump to bottom
Merged
DonJayamanne merged 9 commits into from
Feb 3, 2020
Merged

Ensure we have a base type for all action payloads #9840

DonJayamanne merged 9 commits into from
Feb 3, 2020

Conversation

@DonJayamanne
Copy link

@DonJayamanne DonJayamanne commented Jan 30, 2020

For #9340

  • Ensures all actions have a base type, to pass information such whether the action was invoked to sync state for multiple editors (pointing to same file) in same session or across sessions.
  • This PR only adds the ability to pass such new information to all actions.

@DonJayamanne DonJayamanne added the no-changelog No news entry required label Jan 30, 2020
rchiodo
rchiodo reviewed Jan 30, 2020
View reviewed changes
rchiodo
rchiodo reviewed Jan 30, 2020
View reviewed changes
rchiodo
rchiodo requested changes Jan 30, 2020
View reviewed changes
Copy link

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

🕐

@codecov-io
Copy link

codecov-io commented Jan 30, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (ds/custom_editor@bb95b58). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##             ds/custom_editor    #9840   +/-   ##
===================================================
  Coverage                    ?   60.62%           
===================================================
  Files                       ?      555           
  Lines                       ?    29724           
  Branches                    ?     4479           
===================================================
  Hits                        ?    18020           
  Misses                      ?    10703           
  Partials                    ?     1001
Impacted Files Coverage Δ
src/client/datascience/messages.ts 100% <100%> (ø)
...ience/interactive-common/interactiveWindowTypes.ts 100% <100%> (ø)
src/client/datascience/data-viewing/types.ts 100% <100%> (ø)

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 bb95b58...bf8982f. Read the comment docs.

@DonJayamanne DonJayamanne requested a review from rchiodo January 30, 2020 23:44
rchiodo
rchiodo approved these changes Jan 31, 2020
View reviewed changes
Copy link

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

:shipit:

@DonJayamanne DonJayamanne force-pushed the baseDataForActions branch 2 times, most recently from cb9ebb3 to ae49e64 Compare January 31, 2020 23:01
This was referenced Feb 3, 2020
Closed
Closed
@DonJayamanne DonJayamanne merged commit 666e07d into microsoft:ds/custom_editor Feb 3, 2020
@DonJayamanne DonJayamanne deleted the baseDataForActions branch February 3, 2020 23:21
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 3, 2020

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
No Duplication information No Duplication information

This was referenced Feb 3, 2020
Closed
Merged
DonJayamanne added a commit that referenced this pull request Feb 4, 2020
@DonJayamanne
Remove root level flags used to track cell selection and focus (#9882)
05bf2ea 
For #9340
To be merged after #9840

Basically I got rid of the root level state info that keeps track of the cell that's selected and focused.
This required us to keep the two in sync along with the state at the cell level.
When syncing information between multiple editors this got messy. removing this removes the issues where things could go out of sync, i.e. two ways to storing the same thing. Now we get selected information from the cells directly - single source of truth.
@lock lock bot locked as resolved and limited conversation to collaborators Feb 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@IanMatthewHuff IanMatthewHuff IanMatthewHuff approved these changes

@rchiodo rchiodo rchiodo approved these changes

@DavidKutu DavidKutu Awaiting requested review from DavidKutu

Assignees

No one assigned

Labels

no-changelog No news entry required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DonJayamanne @codecov-io @IanMatthewHuff @rchiodo