Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

return a nil message to get skipped in the pipeline #261

Merged
merged 1 commit into from
Feb 6, 2017

Conversation

jipperinbham
Copy link
Contributor

rather than rely on changing the op type for a message to be skipped, setting it to nil will result in getting skipped here

fixes #260

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 59.812% when pulling f9d120b on 260-drop-noops into df11d32 on master.

@trinchan
Copy link
Contributor

trinchan commented Feb 6, 2017

This LGTM. However I do see a bug in the skipMsg logic. When skipping a message, we don't update the LastMsg, so if we end up using that for persistence it'll be off by a bit.

@jipperinbham
Copy link
Contributor Author

good catch, I think until we start using LastMsg that issue may disappear as the state of persisting transporter state is still influx.

@trinchan
Copy link
Contributor

trinchan commented Feb 6, 2017

Yeah, it should be okay for now and when we spec out persistence, it'll get caught

@jipperinbham jipperinbham merged commit c52fd88 into master Feb 6, 2017
@jipperinbham jipperinbham deleted the 260-drop-noops branch February 6, 2017 22:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

noop message causes panic
3 participants