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.

Sign up for GitHub

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

Jump to bottom

Remove TOX_DEBUG and have asserts always enabled. #381

Merged
merged 1 commit into from
Jan 12, 2017
Merged

Remove TOX_DEBUG and have asserts always enabled. #381

merged 1 commit into from
Jan 12, 2017

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Jan 4, 2017 •
edited
Loading

These are cheap asserts. I've also replaced the fprintf's with
LOGGER_ERROR calls.

This change is Reviewable

@iphydf iphydf added this to the v0.1.3 milestone Jan 4, 2017
@GrayHatter
Copy link

:lgtm: except for the load_state() function... WTF is that?

+@GrayHatter +@nurupo

Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@iphydf iphydf force-pushed the remove-tox-debug branch 2 times, most recently from 229d2da to 4373101 Compare January 5, 2017 12:47
@GrayHatter
Copy link

:lgtm:

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@iphydf iphydf modified the milestones: v0.1.3, v0.1.4 Jan 5, 2017
@GrayHatter
Copy link

:lgtm:

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

@robinlinden
Copy link
Member

:lgtm_strong:

Reviewed 2 of 8 files at r1, 2 of 4 files at r2, 4 of 4 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

@iphydf iphydf modified the milestone: v0.1.4 Jan 10, 2017
@iphydf iphydf force-pushed the remove-tox-debug branch 2 times, most recently from a097e26 to 72cc2e1 Compare January 11, 2017 19:41
@iphydf
Remove TOX_DEBUG and have asserts always enabled.
8b4eae4 
These are cheap asserts. I've also replaced the fprintf's with
`LOGGER_ERROR` calls.
@iphydf
Copy link
Member Author

iphydf commented Jan 11, 2017

@GrayHatter @robinlinden PTAL: merge required additional change: I added another instance of #ifdef TOX_DEBUG in Messenger.c in #392 that I now removed again.

@robinlinden
Copy link
Member

Reviewed 4 of 4 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@GrayHatter
Copy link

:lgtm:

Reviewed 4 of 4 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@iphydf iphydf merged commit 8b4eae4 into TokTok:master Jan 12, 2017
@nurupo
Copy link
Member

nurupo commented Jan 12, 2017

:lgtm_strong:

Comments from Reviewable

@iphydf iphydf deleted the remove-tox-debug branch January 21, 2018 10:34
@iphydf iphydf added refactor Refactoring production code, eg. renaming a variable, not affecting semantics and removed code cleanup labels May 4, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@GrayHatter GrayHatter

Labels
refactor Refactoring production code, eg. renaming a variable, not affecting semantics
Projects
None yet
Milestone
v0.1.4
Development

Successfully merging this pull request may close these issues.
4 participants
@iphydf @GrayHatter @robinlinden @nurupo