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

refactor: netLog directly uses network service #18289

Merged
merged 5 commits into from May 23, 2019

Conversation

@nornagon
Copy link
Member

nornagon commented May 14, 2019

Description of Change

This refactors the netLog API to directly use the netlog interface exposed by the network service, instead of going through NetExportFileWriter. As part of the refactor:

  1. This fixes an issue where calling netLog.startLogging() immediately in app.on('ready') would silently fail to start the netlog.
  2. netLog.startLogging() now returns a promise, which you can await and be sure that the net log has started recording.
  3. netLog.currentlyRecordingPath has been deprecated. If you need access to the currently recording path, you'll have to track it yourself.

Checklist

Release Notes

Notes:

  • Fixed an issue where netLog.startLogging() would silently fail when called immediately during app.on('ready').
  • netLog.startLogging() now returns a promise which resolves when the net log has started recording.
  • Deprecated netLog.currentlyLoggingPath.
@nornagon nornagon requested review from zcbenz and deepak1556 May 14, 2019
@electron-cation electron-cation bot added new-pr 🌱 and removed new-pr 🌱 labels May 14, 2019
@nornagon nornagon marked this pull request as ready for review May 23, 2019
@nornagon nornagon force-pushed the net-log-refactor branch from e881112 to c8bef17 May 23, 2019
Copy link
Member

deepak1556 left a comment

LGTM 👍

couple of code style questions.

atom/browser/api/atom_api_net_log.cc Show resolved Hide resolved
atom/browser/api/atom_api_net_log.cc Outdated Show resolved Hide resolved
@nornagon nornagon requested a review from deepak1556 May 23, 2019
@nornagon nornagon merged commit 646f572 into master May 23, 2019
12 of 13 checks passed
12 of 13 checks passed
build-mac Workflow: build-mac
Details
Artifact Comparison Changes Detected
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
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
build-linux Workflow: build-linux
Details
electron-arm-testing Build #20190523.23 succeeded
Details
electron-arm64-testing Build #20190523.22 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@nornagon

This comment has been minimized.

Copy link
Member Author

nornagon commented May 23, 2019

mac failure is flake; merging.

@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented May 23, 2019

Release Notes Persisted

  • Fixed an issue where netLog.startLogging() would silently fail when called immediately during app.on('ready').
  • netLog.startLogging() now returns a promise which resolves when the net log has started recording.
  • Deprecated netLog.currentlyLoggingPath.
@nornagon nornagon deleted the net-log-refactor branch May 23, 2019
@nornagon nornagon mentioned this pull request May 24, 2019
3 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.