Skip to content
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.

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

fix: return path from netLog.stopLogging #21985

Merged
merged 1 commit into from Jan 31, 2020
Merged

Conversation

nornagon
Copy link
Member

Description of Change

The docs say we return a string, so let's do that. This was broken by the refactor in #18289.

It's a bit weird that we save the path on behalf of the user—all we're doing is keeping track of the value they passed in. That should really be the responsibility of the user, not the API, but since that's how it's documented to work, we might as well make it work the way the docs say (and the way it used to work).

Checklist

Release Notes

Notes: Fixed netLog.stopLogging returning undefined instead of the path to the log.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jan 31, 2020
this._currentlyLoggingPath = null
return _originalStopLogging.call(this)
return _originalStopLogging.call(this).then(() => logPath)
Copy link
Member

Choose a reason for hiding this comment

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

So simple

@nornagon nornagon added the fast-track 🚅 Indicates that this PR is intended to bypass the 24 hour rule. Needs approval from Releases label Jan 31, 2020
@nornagon
Copy link
Member Author

Throwing on [fast-track] to try to get this in before the 8.0.0 stable

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jan 31, 2020
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

LGTM

@MarshallOfSound MarshallOfSound merged commit 1c6d8f5 into master Jan 31, 2020
@release-clerk
Copy link

release-clerk bot commented Jan 31, 2020

Release Notes Persisted

Fixed netLog.stopLogging returning undefined instead of the path to the log.

@trop
Copy link
Contributor

trop bot commented Jan 31, 2020

I have automatically backported this PR to "7-1-x", please check out #21988

@trop trop bot removed the target/7-1-x label Jan 31, 2020
@trop
Copy link
Contributor

trop bot commented Jan 31, 2020

I have automatically backported this PR to "8-x-y", please check out #21989

@sofianguy sofianguy added this to Fixed in 7.1.12 in 7.2.x Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track 🚅 Indicates that this PR is intended to bypass the 24 hour rule. Needs approval from Releases
Projects
No open projects
7.2.x
Fixed in 7.1.12
Development

Successfully merging this pull request may close these issues.

None yet

4 participants