Skip to content
This repository has been archived by the owner on Jun 9, 2018. It is now read-only.

[FR] Fallback Option for Location/ReverseLookup #32

Closed
wants to merge 5 commits into from

Conversation

danner26
Copy link
Contributor

Added support and CLI switch for fallback. If Apple lookup fails, and if the command is specified, it will try to redo the lookup through Google. Likewise, if the Apple reverse lookup fails, and if the command is specified, it will try to redo the reverse lookup through Google.

if args.fallback:
toFallback = True
else:
toFallback = False
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should pull from the config here. pref('Fallback')

@danner26
Copy link
Contributor Author

Whoops, missed that one! Thanks for catching, I updated it.

@danner26
Copy link
Contributor Author

danner26 commented Mar 1, 2018

Did you have a chance to review this? if so, it would be great to have it in the master branch.

@clburlison
Copy link
Owner

@danner26 I just got time to review this. I'm happy with the feature enhancement addition but not the implementation. Using exceptions for core logic handling is pretty dirty. Since I was delayed on the review I've gone ahead and refactored this. Please see #33 and see if you're happy with the changes I've made. If so I'll go ahead with the merged and cut a new patch.

@clburlison clburlison closed this Mar 5, 2018
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.

2 participants