Skip to content

Conversation

@Nightsd01
Copy link
Contributor

@Nightsd01 Nightsd01 commented Sep 25, 2018

• This minor PR removes a few cases of directly using NSLog instead of using our own log function.
• Also changes the name of a target in our SDK project from OneSignal-Dynamic to OneSignal-Static-Framework, since it doesn't produce a dynamic framework.


This change is Reviewable

• Ran a codebase search for usage of NSLog and switched these log statements to use our own log functionality (with different log levels)
• Recently added a new target (OneSignal-Dynamic-Framework). We had an older target called OneSignal-Dynamic, but it actually produced a static framework.
• This commit changes the name OneSignal-Dynamic to OneSignal-Static-Framework to be more accurate/descriptive
@Nightsd01 Nightsd01 requested a review from jkasten2 September 25, 2018 22:49
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

The code changes look good to me.

Do you know if anything could be depending on the name OneSignal-Dynamic today that this rename to OneSignal-Static-Framework could break?

Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @jkasten2)

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, all discussions resolved

• Changed name of a target which the travis script relied on
• Removed the command entirely since it should not even be necessary
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 4 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Nightsd01 Nightsd01 merged commit 8a5aec2 into master Sep 26, 2018
@Nightsd01 Nightsd01 deleted the remove_logs branch September 26, 2018 00:09
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.

3 participants