Skip to content

Models: reformulate the JSON/API content #288

Merged
jayjayjpg merged 3 commits intoember-learn:masterfrom
ppcano:json-api-tweaks
Dec 6, 2018
Merged

Models: reformulate the JSON/API content #288
jayjayjpg merged 3 commits intoember-learn:masterfrom
ppcano:json-api-tweaks

Conversation

@ppcano
Copy link
Contributor

@ppcano ppcano commented Nov 27, 2018

Previously discussed at #240

The PR moves the content of the JSON-API on Ember Data to the Ember Data Flexibility section and remove the rest of the content of the Convention Over Configuration with JSON API.

@jenweber commented:

The JSONAPI section was probably written when the project was brand-new. I have a feeling that we can keep the same content level with fewer words, now that the project is better established and known.

I tried to reformulate the content a bit, but I ended up removing the rest of the Convention Over Configuration with JSON API, feel free anyone to suggest any better form.

@ppcano ppcano changed the title Reformulate the JSON/API content and Models: reformulate the JSON/API content Nov 27, 2018
@MelSumner MelSumner requested a review from a team December 1, 2018 03:22
Copy link
Contributor

@jayjayjpg jayjayjpg left a comment

Choose a reason for hiding this comment

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

This reads great, thank you for the update! I'm just catching up on the thoughts you already left in the previous comment and on discuss to understand if/how the "Convention over Config" section should be moved.

I think it makes sense to run this by some folks on the Ember Data team as well to see what their vision on the placement of that part of the Guides is. What do you think?

this shared store; when they need to display or modify a model, they
first ask the store for it.

## Convention Over Configuration with JSON API
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I'm not sure if this is already common knowledge among front end developers (and especially early learners of JavaScript) and I'm wondering if it might make sense to either keep this paragraph as is or to move this section to another, separate part of the Guides and simply link to it once json-api is mentioned here in the Model Guides, with e.g. something like

By default, Ember Data is designed to work out of the box with [JSON API][json-api].
JSON API is a formal specification for building [...] communicate model data. You can read more about [how embracing the JSON API convention can make the process of building your Ember app more productive here](./link/to/JSON/API/section/moved/from/here).

What are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jessica-jordan

I think this part is useful too. In fact, the content you mentioned in this comment has been moved to the Ember Flexibility section in this PR like:

By default, Ember Data is designed to work out of the box with [JSON API][json-api]
JSON API is a formal specification for building conventional, robust, and performant
APIs that allow clients and servers to communicate model data.

We can extend and/or improve this or any other part, I didn't try more changes because I was not sure how to complete it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now! From what I got from this discussion it indeed makes sense to reduce the content of the Convention over Configuration section, but I think leaving in some more of the prose on the benefits would be useful. I think this following bit is still crucial and could be re-added to the bit about jsonapi that's already part of the Ember Data Flexibility seciton

JSON API standardizes how JavaScript applications talk to servers, so	
you decrease the coupling between your frontend and backend, and have	
more freedom to change pieces of your stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the above paragraph at d88bc73

Personally, my view is that the ED Guides should not promote much the JSON API format and/or any other format because of the situation commented on the previous discussion

Some folks decided to provide a JSON-API backend or endpoints because Ember Data uses it as default format, and this added more work for them.

Anyway, as you suggested this is a feedback for the vision of the ED Guides for the ED team and the current PR looks fine to me because I think it improves a little the guide.

@ppcano
Copy link
Contributor Author

ppcano commented Dec 1, 2018

I think it makes sense to run this by some folks on the Ember Data team as well to see what their vision on the placement of that part of the Guides is. What do you think?

Agree, of course. The initial intention was to share the particular observation and opinion I described on discuss. I am not sure if more folks agree on that and/or see the situation in a different way.

#240 and #288 have been the two PRs addressing the situation.

@jessica-jordan thanks for helping out.

Copy link
Contributor

@jayjayjpg jayjayjpg left a comment

Choose a reason for hiding this comment

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

This reads great, thank you for the swift update ✨

@jayjayjpg jayjayjpg merged commit 845f527 into ember-learn:master Dec 6, 2018
@jayjayjpg jayjayjpg removed their assignment Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants