Skip to content

feat(event processor): Add LogEvent notification support #263

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

Merged
merged 30 commits into from
Oct 4, 2019
Merged

Conversation

jaeopt
Copy link
Contributor

@jaeopt jaeopt commented Sep 9, 2019

  • EventDispatcher sends LogEvent notification if listener is registered.

@jaeopt jaeopt requested a review from a team as a code owner September 9, 2019 16:59
@coveralls
Copy link

coveralls commented Sep 9, 2019

Coverage Status

Coverage decreased (-0.09%) to 95.551% when pulling ebb3aab on jae/epNotif into fdc843f on master.

@jaeopt jaeopt closed this Sep 13, 2019
@jaeopt jaeopt reopened this Sep 13, 2019
@jaeopt jaeopt closed this Sep 17, 2019
@jaeopt jaeopt reopened this Sep 17, 2019
@jaeopt jaeopt closed this Sep 17, 2019
@jaeopt jaeopt reopened this Sep 17, 2019
@jaeopt jaeopt closed this Sep 18, 2019
@jaeopt jaeopt reopened this Sep 18, 2019
@jaeopt jaeopt closed this Sep 19, 2019
@jaeopt jaeopt reopened this Sep 19, 2019
@jaeopt jaeopt closed this Sep 19, 2019
@jaeopt jaeopt reopened this Sep 19, 2019
@jaeopt jaeopt closed this Oct 1, 2019
@jaeopt jaeopt reopened this Oct 1, 2019
Copy link
Contributor

@thomaszurkan-optimizely thomaszurkan-optimizely left a comment

Choose a reason for hiding this comment

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

I had a couple of questions/nits. Needs to pass tests. But, approved! :)

@@ -26,7 +26,6 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
codeCoverageEnabled = "YES"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a part of fixing coverage failure for this PR. I'll try to split into separate one next time :)

@@ -53,8 +53,8 @@ jobs:
branches:
only:
- master
env: SCHEME=OptimizelySwiftSDK-iOS TEST_SDK=iphonesimulator PLATFORM='iOS Simulator' OS=9.3 NAME='iPad Air'
name: PLATFORM='iOS Simulator' OS=9.3 NAME='iPad Air'
env: SCHEME=OptimizelySwiftSDK-iOS TEST_SDK=iphonesimulator PLATFORM='iOS Simulator' OS=12.1 NAME='iPhone 7'
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be in another PR?

@jaeopt jaeopt merged commit 4d26821 into master Oct 4, 2019
@jaeopt jaeopt deleted the jae/epNotif branch October 4, 2019 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants