search

Nexmo / nexmo-cli

Nexmo CLI (Command Line Interface)
https://nexmo.com
MIT License
78 stars 52 forks source link

adding partial app id match for 'nexmo an' #168 #171

Closed Ricool06 closed 5 years ago

Ricool06 commented 5 years ago

Summary

Implements partial app_id matching for nexmo app:numbers to solve #168

Other Information

I wasn't entirely sure how to fit the 2 API calls (one to applicationsList, the other to applicationNumbers) in the existing pattern. I feel like this is the most appropriate way out of the options I was thinking of at the time. Code reviews/suggestions welcome :+1:

AlexLakatos commented 5 years ago

Thanks for the PR! Would you mind making partial app ids work on all commands that use one? I feel the best mechanism for this would be to hold an app id cache so we don't hit the API with every new command, but I'm open to suggestions.

Ricool06 commented 5 years ago

Sure, that sounds better. I'll jump on this tomorrow. Unsure when to reset the cache. How often are a user's apps likely to change. Should I provide a flag to refresh the cache?

mheap commented 5 years ago

If I were implementing this I’d have the cache time out after 5 minutes, or immediately if you’re unable to match the string provided

On 17 Oct 2018, at 10:45, Ricool06 notifications@github.com wrote:

Sure, that sounds better. I'll jump on this tomorrow. Unsure when to reset the cache. How often are a user's apps likely to change. Should I provide a flag to refresh the cache?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

Ricool06 commented 5 years ago

Apologies for the hold up on this, applying for jobs + uni work has led to a busy weekend. This is still on my backlog. :+1:

AlexLakatos commented 5 years ago

Let's merge this for now and we can take care of the other stuff later in a different PR.

Ricool06 commented 5 years ago

Okay, thanks for your patience regardless. University has sort of taken over my life right now.