search

praekelt / go-jsbox-location

Location finder for go-jsbox
Other
2 stars 0 forks source link

Feature/25 add skip to location #34

Closed codiebeulaine closed 8 years ago

codiebeulaine commented 8 years ago

@hodgestar ready for review

hodgestar commented 8 years ago

Couple of minor comments. Otherwise looks good.

codiebeulaine commented 8 years ago

@hodgestar made those changes, I used the skip test I added, is this okay? They all live in the same file now

codiebeulaine commented 8 years ago

@hodgestar also, added the retry option. Having trouble reading the metadata though

hodgestar commented 8 years ago

Tests look good now! Thank you. :)

hodgestar commented 8 years ago

Currently I think if one presses s or r they will work even if the relevant options aren't displayed in the footer? Perhaps we should only add the corresponding handlers if skip_text and retry_text are set?

codiebeulaine commented 8 years ago

@hodgestar ready for review :) again haha

hodgestar commented 8 years ago

Looks good!

We need two extra tests that test that Skip and Retry are displayed in the menus (currently we test that pressing s and r does the right thing).

hodgestar commented 8 years ago

:+1: