Skip to content

Delete all experimentVSCode branches from the code #3283

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
merged 3 commits into from
Feb 14, 2020
Merged

Delete all experimentVSCode branches from the code #3283

merged 3 commits into from
Feb 14, 2020

Conversation

@MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented Jan 7, 2020

Because experimentVSCode is always true

MichaelDeBoey
MichaelDeBoey commented Jan 7, 2020
View reviewed changes
Copy link
Contributor Author

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

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

onKeyDown & onKeyUp aren't used anymore so they could be deleted too

packages/app/src/app/overmind/effects/keybindingManager.ts Outdated
window.removeEventListener('keyup', onKeyUp);
isStarted = false;
},
start() {},
Copy link
Contributor Author

@MichaelDeBoey MichaelDeBoey Jan 7, 2020

Choose a reason for hiding this comment

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

Since _overmindInstance.state.preferences.settings.experimentVSCode is always true, we'll always instantly return.
So we don't need the body at all

packages/app/src/app/overmind/effects/keybindingManager.ts Outdated
isStarted = false;
},
start() {},
pause() {},
Copy link
Contributor Author

@MichaelDeBoey MichaelDeBoey Jan 7, 2020

Choose a reason for hiding this comment

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

Since there aren't any eventListeners added, we don't need to remove them.

@lbogdan lbogdan temporarily deployed to pr3283 January 7, 2020 17:41 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Jan 7, 2020 •
edited
Loading

Build for latest commit 37125b0 is at https://pr3283.build.csb.dev/s/new.

@lbogdan lbogdan temporarily deployed to pr3283 January 7, 2020 18:51 Inactive
christianalfoni
christianalfoni approved these changes Jan 9, 2020
View reviewed changes
Copy link
Contributor

@christianalfoni christianalfoni left a comment

Choose a reason for hiding this comment

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

This looks good to me!

Did a quick runthrough of editor and dashboard.

Want a comment form @CompuIves here before we merge though :)

@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from 3a20334 to a888fac Compare January 16, 2020 16:47
@lbogdan lbogdan temporarily deployed to pr3283 January 16, 2020 16:53 Inactive
@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Jan 27, 2020

Ping @CompuIves @christianalfoni

@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from a888fac to 730de9f Compare January 31, 2020 13:35
@lbogdan lbogdan temporarily deployed to pr3283 January 31, 2020 13:42 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from 730de9f to a85a898 Compare February 5, 2020 16:36
@lbogdan lbogdan temporarily deployed to pr3283 February 5, 2020 16:42 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from a85a898 to ad2612f Compare February 5, 2020 23:33
@lbogdan lbogdan temporarily deployed to pr3283 February 5, 2020 23:39 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from ad2612f to 7463a3b Compare February 6, 2020 00:07
@lbogdan lbogdan temporarily deployed to pr3283 February 6, 2020 00:13 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from 7463a3b to eb180f8 Compare February 8, 2020 12:34
@lbogdan lbogdan temporarily deployed to pr3283 February 8, 2020 12:40 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from eb180f8 to 7d65d92 Compare February 13, 2020 14:10
@lbogdan lbogdan temporarily deployed to pr3283 February 13, 2020 14:16 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from 7d65d92 to d766e4b Compare February 13, 2020 18:31
@lbogdan lbogdan temporarily deployed to pr3283 February 13, 2020 18:39 Inactive
@MichaelDeBoey MichaelDeBoey force-pushed the fix-experimentVSCode-branches branch from d766e4b to 37125b0 Compare February 13, 2020 21:38
CompuIves
CompuIves approved these changes Feb 14, 2020
View reviewed changes
Copy link
Member

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

Very nice cleanup! Well written!

@CompuIves CompuIves merged commit febd1fe into codesandbox:master Feb 14, 2020
@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Feb 14, 2020

Always happy to help out 🙂

@MichaelDeBoey MichaelDeBoey deleted the fix-experimentVSCode-branches branch February 14, 2020 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CompuIves CompuIves CompuIves approved these changes

@christianalfoni christianalfoni christianalfoni approved these changes

Assignees

No one assigned

Labels

🔨 Refactor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MichaelDeBoey @lbogdan @CompuIves @christianalfoni