Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Dec 15, 2022. It is now read-only.
/githubPublic archive

Update shell.openExternal to promise due to electron update on atom#2625

Merged
smashwilson merged 2 commits intomasterfromupdate-electron-apis
Feb 12, 2021

Conversation

sadick254
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

Description of the Change

There are some Electron APIs that changed to only return Promises (and not run callbacks passed to them), mostly as of Electron 7+. Updating to be compatible with those would be great for updating Atom's Electron version.

Screenshot or Gif

Applicable Issues

#2624

@codecov
Copy link

codecovbot commentedFeb 11, 2021
edited
Loading

Codecov Report

Merging#2625 (4ab44f3) intomaster (044931d) willincrease coverage by0.01%.
The diff coverage is94.11%.

Impacted file tree graph

@@            Coverage Diff             @@##           master    #2625      +/-   ##==========================================+ Coverage   93.45%   93.46%   +0.01%==========================================  Files         237      237                Lines       13234    13215      -19       Branches     1906     1900       -6     ==========================================- Hits        12368    12352      -16+ Misses        866      863       -3
Impacted FilesCoverage Δ
lib/controllers/issueish-searches-controller.js89.47% <50.00%> (+8.52%)⬆️
lib/controllers/issueish-list-controller.js77.77% <100.00%> (-2.23%)⬇️
lib/controllers/remote-controller.js100.00% <100.00%> (ø)
lib/views/actionable-review-view.js100.00% <100.00%> (ø)
lib/views/issueish-link.js81.25% <100.00%> (-2.09%)⬇️
lib/atom/gutter.js92.85% <0.00%> (+2.38%)⬆️

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update044931d...4ab44f3. Read thecomment docs.

@sadick254sadick254force-pushed theupdate-electron-apis branch 2 times, most recently fromecf308b tod08bb7bCompareFebruary 11, 2021 06:23
Copy link
Contributor

@smashwilsonsmashwilson left a comment

Choose a reason for hiding this comment

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

Hmm CodeCov is complaining about these... they must not be covered by existing tests. I think that's probably not worth addressing here, though, because these methods don't do anything but call two other functions that we'd need to stub anyway.

@smashwilson
Copy link
Contributor

Looks like we'll need to update the stubbing in some tests, too.

sadick254 reacted with thumbs up emoji

@sadick254sadick254force-pushed theupdate-electron-apis branch 4 times, most recently from5e165e6 to9678921CompareFebruary 12, 2021 07:09
Copy link
Contributor

@smashwilsonsmashwilson left a comment

Choose a reason for hiding this comment

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

it('calls shell.openExternal with specified url',asyncfunction(){
constwrapper=shallow(buildApp());
sinon.stub(shell,'openExternal').callsArg(2);
sinon.stub(shell,'openExternal').callsFake(()=>{});
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these could also be.returns(Promise.resolve()). No big deal though.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers
1 more reviewer

@smashwilsonsmashwilsonsmashwilson approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@sadick254@smashwilson

[8]ページ先頭

©2009-2025 Movatter.jp