Skip to content
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

Update the Super Rentals Tutorial to use the Request Service #238

Merged
merged 10 commits into from
Jul 6, 2024

Conversation

Baltazore
Copy link
Contributor

Fixes #232

Brings updates to Chapter 11 of tutorial.

Copy link
Member

@IgnaceMaes IgnaceMaes left a comment

Choose a reason for hiding this comment

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

Should we include a paragraph on which versions the new request service has been available in Ember Data?

That seems useful for people coming back to the tutorial having previous experience with Ember Data.

@Baltazore
Copy link
Contributor Author

Baltazore commented Jun 27, 2024

@IgnaceMaes can you please take another look at this one?

src/markdown/tutorial/part-2/11-ember-data.md Outdated Show resolved Hide resolved
src/markdown/tutorial/part-2/11-ember-data.md Outdated Show resolved Hide resolved
src/markdown/tutorial/part-2/11-ember-data.md Outdated Show resolved Hide resolved
src/markdown/tutorial/part-2/11-ember-data.md Show resolved Hide resolved
Baltazore and others added 8 commits July 5, 2024 21:41
This change is needed to avoid confusion around model names.
Previosly JsonApiSerializer was singularizing type of every json-api
response object, so convention "model name should be singular always
was met.
In new world, we don't do automatic conversion of payloads and try to
have them as close as possible to network response. So if application
developers do not manually add singularization of json-api response
objects type - it will never happen. Therefore expected model name would
be plural, or in better words match type from payload.
Co-authored-by: Chris Thoburn <runspired@users.noreply.github.com>
Co-authored-by: Krystan HuffMenne <kmenne+github@gmail.com>
@IgnaceMaes
Copy link
Member

Looking good!

The code style comments are minor details, but I believe it's important to consciously think about how we handle these. Users see the guides as the recommended way of structuring an app and will take over this style.

Copy link
Member

@IgnaceMaes IgnaceMaes left a comment

Choose a reason for hiding this comment

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

Awesome job!

Excited for this one 🚀

@IgnaceMaes IgnaceMaes merged commit f766728 into ember-learn:main Jul 6, 2024
5 checks passed
@IgnaceMaes
Copy link
Member

IgnaceMaes commented Jul 6, 2024

Looks like the code block languages and filenames are not set.

@Baltazore could you look into opening a new PR to add these?

preview: https://deploy-preview-2046--ember-guides.netlify.app/release/tutorial/part-2/ember-data/#toc_working-with-request-builders-and-handlers

@Baltazore
Copy link
Contributor Author

Looks like the code block languages and filenames are not set.

@Baltazore could you look into opening a new PR to add these?

preview: https://deploy-preview-2046--ember-guides.netlify.app/release/tutorial/part-2/ember-data/#toc_working-with-request-builders-and-handlers

Sure will do, what are the expectations for filenames to show up? Any ideas why it does not work ?

@IgnaceMaes
Copy link
Member

Sure will do, what are the expectations for filenames to show up? Any ideas why it does not work ?

Looking at the other examples, it seems like the files need to be added to git. When looking into the source files there are a couple of code blocks that are passed a flag hidden, with git add commands.

That would be my guess, but I'm still in the process of figuring out this tutorial builder app too, heh 😄

@IgnaceMaes
Copy link
Member

Nevermind, that was done I see just now. It looks like the lang attributes were missing.

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.

Update the Super Rentals Tutorial to use the Request Service
4 participants