Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign uprefactor: make shell.OpenExternal async #17135
+69
−139
Conversation
codebytere
requested a review
from
as a
code owner
Feb 26, 2019
electron-cation
bot
added
the
new-pr 🌱
label
Feb 26, 2019
codebytere
changed the title
refactor: make OpenExternal async on win
refactor: offload OpenExternal to new thread on Windows
Feb 26, 2019
codebytere
changed the title
refactor: offload OpenExternal to new thread on Windows
refactor: offload shell.OpenExternal to new thread on Windows
Feb 26, 2019
codebytere
force-pushed the
openexternal-async
branch
2 times, most recently
from
12bc3eb
to
f0da519
Feb 26, 2019
deepak1556
requested changes
Feb 26, 2019
atom/common/platform_util_win.cc Outdated
atom/common/platform_util_win.cc Outdated
electron-cation
bot
removed
the
new-pr 🌱
label
Feb 27, 2019
codebytere
force-pushed the
openexternal-async
branch
4 times, most recently
from
94d73e7
to
bb6b06e
Mar 1, 2019
codebytere
requested a review
from
deepak1556
Mar 4, 2019
codebytere
force-pushed the
openexternal-async
branch
3 times, most recently
from
45d6454
to
0d8a16e
Mar 4, 2019
This comment has been minimized.
This comment has been minimized.
|
Do we want to support both |
This comment has been minimized.
This comment has been minimized.
|
We can either deprecate the sync API or just leave a fake sync API. I'm good with either, the async API was fake anyway. |
codebytere
force-pushed the
openexternal-async
branch
from
f6610f3
to
1372f49
Mar 6, 2019
codebytere
changed the title
refactor: offload shell.OpenExternal to new thread on Windows
refactor: make shell.OpenExternal async
Mar 6, 2019
codebytere
force-pushed the
openexternal-async
branch
3 times, most recently
from
899e7c9
to
49f9bf8
Apr 5, 2019
nornagon
reviewed
Apr 5, 2019
atom/browser/atom_browser_client.cc Outdated
codebytere
force-pushed the
openexternal-async
branch
from
49f9bf8
to
e5ba8f6
Apr 5, 2019
deepak1556
requested changes
Apr 5, 2019
codebytere
requested a review
from
deepak1556
Apr 5, 2019
deepak1556
requested changes
Apr 5, 2019
atom/common/platform_util_win.cc Outdated
atom/common/platform_util_win.cc Outdated
codebytere
force-pushed the
openexternal-async
branch
from
ca8ca8b
to
63f641c
Apr 5, 2019
deepak1556
requested changes
Apr 5, 2019
atom/common/platform_util_win.cc Outdated
atom/common/platform_util_win.cc Outdated
codebytere
force-pushed the
openexternal-async
branch
from
63f641c
to
6fa1d61
Apr 5, 2019
codebytere
force-pushed the
openexternal-async
branch
12 times, most recently
from
7939392
to
a57c728
Apr 26, 2019
codebytere
force-pushed the
openexternal-async
branch
from
a57c728
to
6a28841
May 3, 2019
This comment has been minimized.
This comment has been minimized.
|
@deepak1556 this should finally be ready for final review! |
jkleinsc
requested a review
from
deepak1556
May 3, 2019
deepak1556
approved these changes
May 3, 2019
|
LGTM, will there be a deprecation notice for the sync api in a separate PR for 6-0-x ? |
codebytere
merged commit 6d96f30
into
master
May 3, 2019
6 checks passed
Semantic Pull Request
ready to be squashed
Details
appveyor: win-ia32-testing
AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr
AppVeyor build succeeded
Details
appveyor: win-x64-testing
AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr
AppVeyor build succeeded
Details
release-notes
Release notes found
This comment has been minimized.
This comment has been minimized.
release-clerk
bot
commented
May 3, 2019
|
No Release Notes |
codebytere
deleted the
openexternal-async
branch
May 3, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
codebytere commentedFeb 26, 2019
•
edited
Description of Change
Offload
OpenExternalto new thread on Windows.cc @deepak1556 @zcbenz
Checklist
npm testpassesRelease Notes
Notes: none