gma / nesta

File Based CMS and Static Site Generator
http://nestacms.com
MIT License
902 stars 120 forks source link

Fix issue generating a new plugin #153

Closed jkowens closed 9 years ago

jkowens commented 9 years ago

The nesta plugin:create command fails when attempting to generate the init.rb file due to a missing directory. I added a method call to create the needed directory as well refactored the tests a bit.

gma commented 9 years ago

Thanks for reporting this. I thought it was a bit odd when I first saw it, but didn't work out what was going on until I dug in and tried it for myself.

Obviously, generating plugins used to work. I tried generating one myself, and scratched my head for a moment until I saw that the gem that had been generated for my test plugin contained the path lib/nesta/plugin/foobar. What it ought to contain (and what was missing) is a folder called lib/nesta-plugin-foobar.

We use Bundler's bundle gem command to create a skeleton Ruby gem. In their infinite wisdom, Bundler's maintainers have changed its behaviour. It now enforces a naming convention requiring that dashes in a gem's name will be converted into a corresponding directory hierarchy inside the gem's root directory.

Here's the commit: bundler/bundler@eeba36b1fb2f142fafc05bdbdc6eb03c836a32d4

They've taken the idea from the RubyGems naming conventions: http://guides.rubygems.org/name-your-gem/

That it's a recommendation rather than a requirement didn't seem to get considered in the patch; I'm not seeing an option that will allow us to keep the old behaviour. "LOL Ruby" springs to mind…

Our options:

  1. Refactor Nesta so it can load plugins located in these new paths.
  2. Generate our skeleton plugins some other way.

Right now I'm not sure which I favour, but I haven't time to look into it further this weekend.

jkowens commented 9 years ago

I decided to take a stab at making a way to create and load plugins with the new bundler conventions and still maintain compatibility with existing plugins. Does this seem like an ok solution?

gma commented 9 years ago

I was wondering about that approach too, and haven't come up with any reasons why that wouldn't solve it nicely. Go for it…

jkowens commented 9 years ago

I've updated the commit, let me know if you see any issues :smiley:

philmill commented 9 years ago

Would we want to ensure that developers are using the updated version of Bundler. I'm guessing if @jkowens has solved the backwards compatibility issue, than we wouldn't need it. Otherwise s.add_development_dependency('bundler', '>= 1.0.1') in nesta.gemspec gets us to the version with the updated directory structure.

Nice work @jkowens :+1:

gma commented 9 years ago

I hear what you're saying Phil, but I don't think software like Nesta should depend on a specific version of a package management tool. Ideally, you should be able to work with Nesta without having bundler installed at all.

I've been thinking about this issue today, and I've realised that Nesta shouldn't need to get more complicated because another project has an opinion on how to structure a gem. Nesta's current opinion on the directory structure is equally valid.

I'll look at generating the gem within Rails (in the original format) and see how complicated that is. The rubygem format isn't a moving target, but bundle gem could change again next week.

philmill commented 9 years ago

@gma I too hear you and when trying to solve this myself, I have the dev dependency commented out. I figured you would have added it if you felt Nesta needed it. Seems totally reasonable to leave it out, but I also cannot remember the last time I started a Ruby project without Bundler, maybe back in the RVM gemset days.

Wow, my first time seeing houndci. Not impressed. Single quoted strings are great!

gma commented 9 years ago

@philmill I'd totally forgotten I'd setup Hound CI until it started complaining about the only sane way to use single/double quoted strings. I took it out back and shot it.

@jkowens I'm really sorry you did all this work Jake, and we didn't end up using it. If it's any consolation, had you not done this Nesta 0.11.0 probably still wouldn't be released, as I'd have had yet another job to do (i.e. working this patch out for myself) before I'd have realised that I'd prefer for Nesta's code not to need to know how to read two gem formats. I don't know if you saw it, but we've now got a bunch of templates in the templates/plugins folder that nesta plugin:create uses to build a gem. This wasn't the first time that changes in Bundler had broken plugin generation.

I've also added you both to the /credits page on the web site…

gma commented 9 years ago

@jkowens And now I've just noticed I've been calling you Jake all this time. Shit, sorry! Fixing it…

jkowens commented 9 years ago

@gma Haha thanks for the credit! I like your solution, it was a good call :+1: I'm glad I was able to help in some way. I've got to learn to open up discussion before jumping into code :)