varvet / godmin

Admin framework for Rails 5+
http://godmin-sandbox.herokuapp.com
MIT License
486 stars 47 forks source link

Added option to skip updating navigation links partial #190

Closed telamon closed 8 years ago

telamon commented 8 years ago

Hey!

I ran into this issue where I wanted to use godmin inside an engine that does not provide it's own views/shared/_navigation.html.erb and trying to use the godmin:resource generator always failed at trying to manipulate the non-existing file.

So basically I've just added an option to the generator in order to skip that step entierly. The default behavior should be unaffected.

Linuus commented 8 years ago

Thank you for the PR @telamon

It looks ok to me, I just have a few thoughts.

@telamon Can you fix the issues above and squash the commits to a single commit? It would also be great if you can rebase onto master so Travis doesn't fail.

Thanks! :+1:

jensljungblad commented 8 years ago

I agree with changing the name to skip-navigation.

Other than that, looks great. Thanks!

telamon commented 8 years ago

Squashed it and left the documentation in the commit. ( I find myself at a loss on where to extend the readme with this information )

I realize that there's still a potential bug in the generator, it's easy to check if the _navigation.html.erb exists , but what should the default behavoiur be? Auto-create it or auto-skip it?

jensljungblad commented 8 years ago

@telamon You're right, we should probably check that the template exists before adding something to it. As for what the behavior should be if it's missing.. Good question. I think I'm leaning towards doing nothing. If someone removes the file they probably don't want it re-created.

jensljungblad commented 8 years ago

Hey @telamon, you wanna finish this PR?