search

PileProject / drive

The drive project
Other
5 stars 4 forks source link

Refactor/settings #22

Closed mandaiy closed 8 years ago

mandaiy commented 8 years ago

yet another pull req for the setting activity.

breaking changes:

there are still many hard-coding strings, incoherent naming. please comment

mandaiy commented 8 years ago

I'd like to discuss

tiwanari commented 8 years ago

Question, during search, can users choose a device or go to other page (I mean "can the process stop correctly?") BluetoothDevice seems good. Is there any problem ?

mandaiy commented 8 years ago

Question, during search, can users choose a device or go to other page (I mean "can the process stop correctly?")

Of course they can.

BluetoothDevice seems good. Is there any problem ?

there's no problem, but I need your approval ;) maybe this naming scheme is suitable, since there could be 'WifiDevice' or something. or it could be 'BluetoothMachine'. which is preferable?

tiwanari commented 8 years ago

Ok, I also think we should discuss about using "machine" or "device," and in my opinion, machine is better to keep code consistence. I have one more question, what should users do when they try to search again?

mandaiy commented 8 years ago

Ok, I also think we should discuss about using "machine" or "device," and in my opinion, machine is better to keep code consistence.

👍

I have one more question, what should users do when they try to search again?

Now this app continually scans bluetooth devices (in other words, users do not need to restart it) To be precise, the scanning restarts when it ends

https://github.com/myusak/drive/blob/30cffb6bb15e0f8add521d8e3645301af4db78c3/app/src/main/java/com/pileproject/drive/setting/machine/BluetoothDeviceSelectFragment.java#L323-L347

When the broadcast receiver receives 'BluetoothAdapter.DISCOVERY_FINISHED', 'onNext' is called. Then, it calls BluetoothAdapter#startDiscovery() again.

tiwanari commented 8 years ago

Perfect :)

mandaiy commented 8 years ago

I hope you accept my pull request.

tiwanari commented 8 years ago

thanks for a notification:) give me time for check.

tiwanari commented 8 years ago

And this PR seems to conflict with the previous PR. Please fix it.

tiwanari commented 8 years ago

Though there are some kinds of stuff to be fixed, totally LGTM. APPROVED.

mandaiy commented 8 years ago

What are 'some kinds of stuff to be fixed'? It sounds like the PR contains more issues other than you mentioned. I'd like to fix these points.

tiwanari commented 8 years ago

I'm sorry for confusing you... I mean "there are kinds of some stuff to be fixed (as I mentioned above)". So, all are fixed and let's close this PR. I love it:)