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

Patch 4 #1890

Closed
wants to merge 3 commits into from
Closed

Patch 4 #1890

wants to merge 3 commits into from

Conversation

toraritte
Copy link
Contributor

Thank you for all the great work!

See https://github.com/elixir-ecto/ecto/blob/master/lib/mix/ecto.ex#L37

Although there would be no compilation error because of Mix.config/3
but it wouldn't result in a proper behaviour.
This would probably be helpful for someone like me, who never
heard of embedded data models. Unfortunately, I couldn't find
a DB-agnostic description (or maybe it runs by another name).
@@ -355,7 +355,7 @@ defmodule Ecto do

### Embeds

Ecto also supports embeds. While associations keep parent and child
Ecto also supports [embeds](https://docs.mongodb.com/manual/core/data-model-design/#data-modeling-embedding). While associations keep parent and child
Copy link
Member

Choose a reason for hiding this comment

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

Embeds are not specific to mongo, so I don't think we should link exclusively to mongo. :)

Copy link
Contributor Author

@toraritte toraritte Jan 3, 2017

Choose a reason for hiding this comment

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

Yeah, I had my concerns about that too but I couldn't find any DB-agnostic description of it (except for this one but I have too little experience to make sure that this is actually the same topic).
Thanks again and happy new year!

@josevalim
Copy link
Member

I have added one comment, thank you!

@@ -389,7 +389,7 @@ defmodule Ecto do
Ecto requires developers to specify the key `:ecto_repos` in their application
configuration before using tasks like `ecto.create` and `ecto.migrate`. For example:

config :my_app, :ecto_repos, [MyApp.Repo]
config :my_app, ecto_repos: [MyApp.Repo]
Copy link
Member

Choose a reason for hiding this comment

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

Btw, those two calls are equivalent. Both would work. :)

@josevalim
Copy link
Member

Merged one commit manually, thank you!

@josevalim josevalim closed this Jan 4, 2017
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.

2 participants