Skip to content

Simplifying matching routes and constructing responses #6

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

Merged
merged 7 commits into from
Aug 24, 2024

Conversation

sv3ndk
Copy link
Contributor

@sv3ndk sv3ndk commented Aug 18, 2024

Hi,

I'm reading your book, I really like it!

I'm a rust beginner. While coding along at home and digging into what means what, I stumbled upon some possible code tweakings for chapter 3, which I'm sending below in case they might interrest you.

One last aspect that was puzzling me was that sometimes the code is returning the output of generate_api_response(() directly, whereas sometimes it's unrapping it with ? then immediately re-bundling it into another Ok. Maybe there's some error we want to let bubble up in some cases that I don't understand, so I didn't dare to touch that part 😄

I hope you find some of this useful, let me know your thoughts!

@lmammino
Copy link
Contributor

Hello, @sv3ndk!

Thank you so so much for not just buying a copy of the book but also for taking the time to submit a PR. This is super valuable for us and really motivating! ❤️

We'll do a proper review next week and try to get this merged in some form (plus adjust the code examples in the book to reflect the changes). At a first glance I like some of the suggestions, but I think there's some more fine tuning we can do. 😉

Thanks once more and have a great Sunday! 🌞

@lmammino
Copy link
Contributor

Hello, @sv3ndk.

Sorry, this took us a little bit longer than anticipated.
I really liked the idea of using the status constants and applied that everywhere.

Your PR also made me realise that it might make more sense to have more util functions for all the different kinds of responses:

  • A JSON response for a generic Serializable struct
  • A response with an empty payload (to pass a specific error status code back to the user)
  • A response for redirects

So I refactored the utils library and used this idea in the lambda code.

I'll propagate these changes in the code for all the other chapters and get this merged.

Thanks once again for your contribution! 🤝

@lmammino lmammino changed the title Chap03 suggested code tweakings Simplifying matching routes and constructing responses Aug 24, 2024
@lmammino lmammino merged commit b4a32da into rust-lambda:main Aug 24, 2024
@sv3ndk
Copy link
Contributor Author

sv3ndk commented Aug 24, 2024

Hi,

Thanks for the very kind and detailed feed-back. Those updates look great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants