Skip to content
This repository has been archived by the owner on Nov 17, 2020. It is now read-only.

write seeds for comments and dependencies #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Polo2
Copy link
Contributor

@Polo2 Polo2 commented Sep 27, 2018

Some seeds to start using our toolbox.

@michaelbaudino michaelbaudino temporarily deployed to toolbox-staging-pr-20 September 27, 2018 15:18 Inactive
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I'm not in favor to add those to db/seeds.rb which I would keep for "real"
data that is essential for running the application.

But if we find them useful, I'm not against adding this as a script in bin
directory, as a rake task, or maybe another way?

db/seeds.rb Outdated
@@ -0,0 +1,18 @@
Dependency.destroy_all
Comment.destroy_all
Copy link

Choose a reason for hiding this comment

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

Whether it's "real" seeds, or fake data we would find useful in a development
workflow, I would not do this, we should never destroy all records for a model
in seeds IMO.

db/seeds.rb Outdated
Dependency.destroy_all
Comment.destroy_all

puts "create 2 dependencies: d1 and d2"
Copy link

Choose a reason for hiding this comment

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

I would remove this. If we want more info about seeds, we can just read this
file, and when we execute the db:seed task, rails logs are printed on
standard output if I recall correctly.

db/seeds.rb Outdated
Comment.destroy_all

puts "create 2 dependencies: d1 and d2"
d1 = Dependency.create(name: "Gem n°1")
Copy link

Choose a reason for hiding this comment

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

I'd recommend Dependency#find_or_create_by!, so that it's created only once,
and fail directly when the creation is not possible.

db/seeds.rb Outdated

puts "create 2 published comments; c1 and c2, related to dependency d2"
c1 = Comment.create(body: "published comment number 1", published: true, dependency: d2)
c2 = Comment.create(body: "published comment number 2", published: true, dependency: d2)
Copy link

Choose a reason for hiding this comment

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

Local variables c0, c1, c2 are dead code (unused) I think.

db/seeds.rb Outdated
puts "#{Dependency.count} created"
puts "#{Comment.count} created"

puts "Happy Growing !"
Copy link

Choose a reason for hiding this comment

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

As said, I'd remove all #puts calls. But we miss a final \n at end of file
here.

@ghost
Copy link

ghost commented Sep 28, 2018

Another remark: we must keep in mind that if we add this, we'll have to maintain it.

@Polo2 Polo2 temporarily deployed to toolbox-staging-pr-20 September 28, 2018 10:25 Inactive
@Polo2 Polo2 requested a review from a user September 28, 2018 10:43
@Polo2
Copy link
Contributor Author

Polo2 commented Sep 28, 2018

I moved this behavior on a rake task db:dev_seeds, thus:

  • it's not about what we expect about seeds.rb should no or not
  • it's only a dev script to clean database and keep a few starting datas

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

Successfully merging this pull request may close these issues.

2 participants