search

Antidote-for-Tox / objcTox

No longer maintained
Mozilla Public License 2.0
37 stars 12 forks source link

Change pause logic to use bitmask #67

Closed Chuongv closed 9 years ago

Chuongv commented 9 years ago

Let me know if I missed anything.

dvor commented 9 years ago

Have you finished with this pull request or it is in progress?

(if you like you can edit title to be WIP: Change pause logic to use bitmask for not-ready pull requests)

Chuongv commented 9 years ago

I thought I updated it. Is it still showing for you "WIP"?

dvor commented 9 years ago

No, it isn't "WIP". Will review it then. :-)

Chuongv commented 9 years ago

Yea it should be ready now. I had missed a few edge cases when I was testing earlier. But should be good now.

dvor commented 9 years ago

Hm, OCTSubmanagerCalls is getting complicated. Different states are changed in lots of places. You don't mind if I'll try to refactor it a little bit? It would be great if you could provide more unit tests so I'll be sure I won't break anything.

Chuongv commented 9 years ago

Hm, OCTSubmanagerCalls is getting complicated. Different states are changed in lots of places. You don't mind if I'll try to refactor it a little bit? It would be great if you could provide more unit tests so I'll be sure I won't break anything.

Feel free to do so :smile: I don't have any unit tests done yet, but you can use https://github.com/Antidote-for-Tox/Antidote/tree/chuongv/improveSwitchingBetweenCalls in the meantime to test. Just have to modify the podfile to point to the correct remote branch of objcTox.

I'll work on the unit tests in the meantime.

Chuongv commented 9 years ago

Closing this one in favor of https://github.com/Antidote-for-Tox/objcTox/pull/68

I won't be making any changes if you are going to refactor. If not, let me know and I can look into refactoring the logic, since it is a little messy.