-
Notifications
You must be signed in to change notification settings - Fork 127
Add helper methods to request class #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jthomerson
commented
Jan 17, 2019
@jeremydaly I wasn't sure the best way to add tests for these methods due to #96. Also, I wasn't sure if you'd want other helper methods. I'm going to integrate this branch into my app and test it there (so consider this a WIP). Please provide feedback on naming / other helper methods / test methodology and I can update this PR.
coveralls
commented
Jan 17, 2019
Pull Request Test Coverage Report for Build 224
💛 - Coveralls |
jeremydaly
commented
Jan 17, 2019
Thanks for this @jthomerson. Naming conventions seem fine, but I still wavering on the asArray functionality. The only reason I added it for some of the Response methods was for backwards compatibility, but I'm wondering if new features should standardize on returning arrays by default. My other thought was to add convenience methods, like getHeaderAsArray(), but I don't want to bloat the project. What are you thoughts?
I'm not sure how many of these we need, either. Like the getQuery() seems like it might make sense. But I don't think we'd want to add things like getVersion() and getRoute() as we are simply exposing these as properties. That seems like overkill.
I'm open to suggestions though.
jthomerson
commented
Jan 17, 2019
Thanks for the quick response. Yeah, I wasn't sure about the asArray part either, but added it to try to be consistent with the response methods. I definitely think that the most common usecase is to get a single value - not an array. So I wouldn't want to see arrays returned by default.
I think a separate function that returns arrays if you want it would be better than a flag because then you could actually take advantage of return types to know you're getting a string vs an array. (that thought just made me realize I forgot to update the t.js ... which leads me to this question: curious your reasoning on developing it in JS and exposing TS files vs just developing it in TS)
Somewhat separately: I had a crude bunch of utilities that we'd been using for all of our APIs, and I'm investigating getting rid of them to use only your Lambda API lib. Based on this first conversion, I'm going to have a laundry list of suggestions on the API, documentation, etc. What's your preferred way of getting those?
No description provided.