Refactor project

[e2]

• use Guard::Compat
• setup Guard::RSpec
• setup Guard::RuboCop
• fix RuboCop offenses
• break up main class into smaller (by responsibilities)
• rebase to imathis/master

[imathis]

:boom: That's some serious refactoring. It looks really good! I'll test it out some in the morning. Is this good to go, or still a work in progress?

[e2]

@imathis - everything should work fine. The tests don't provide 100% coverage, but they are probably good enough - and the tests are green: https://travis-ci.org/guard/guard-jekyll-plus/builds/44173500.

I'm pretty much done (the goal was to use Guard::Compat to make Guard::Jekyllplus more immune to current and future internal changes in Guard). I tried to keep the functionality 100% the same, so there shouldn't be any surprises.

A few issues maybe:

• changing the config files sometimes results in very long restart periods for the server (only Jekyll/Webrick server AFAIR) - maybe about 10 seconds even. I'm not sure if this is due to Webrick shutting down in a strange way or Celluloid waiting for dead thread. I didn't investigate, because it seems a minor issues - changing configs won't happen that often, and since it works quite well now, that's good enough. Besides, I think using Thin side-steps the issues (seemed to restart really, really fast)
• I made it so the "silent" option affects the Rack environment to silence Thin - but this could be done in a better way
• the config file support isn't tested well.
• the removing of files should have worked as it did - I'm not too sure about the "check for at least one file existing"
• I reorganized things in the Gemfile+gemspec since the responsibilities of each are very confusing
• there's a dependency on Guard 2.10.3 (which was just released), although this may not be necessary - in general it's not a good idea to watch the whole project directory and ignore _site (it's better to separate things into _site and src dirs, and watch only src - but then _config.yml isn't watched). The ignore rule in your template Guardfile was ignored in some cases and this cause and inifinite loop of triggered changes. Long story short: if you want Guard to respond to files in the root project directory, it's best to use symlinks.
• the Guard setup is still a bit confusing, because of the "hack" of "watching everything", ignoring _site and internally using the default extensions and now also including jekyll excludes - it may be hard for users to understand what to put in a Guardfile and where if they don't have the default setup. And how configs are resolved were also confusing to me.
• the debugging/verbose stuff could be improved, although it's a lot better now
• instead of testing for Rack to enable it, I'd just set e.g. rack: true or something and expect things to crash and fail if Rack doesn't work.

But everything seems to work fine right now. They only thing I'd maybe add is coveralls/coverage - so you can see which areas aren't tested with tests, fix that and have more confidence when releasing.

Otherwise if something doesn't work - adding a test should be trivial. So that should help with contributions and further refactoring - and reduces the need to test stuff manually.

If I were you I'd notify other people with forks about the changes - I'm sure they'll have plenty of ideas for improvements and tweaks.

Either way - I'm done until I break something in Guard again. I'd probably file an issue with Jekyll so that fork() wouldn't be necessary - but I prefer Rack+Thin anyway.

[bvsatyaram]

+1 Start the game already!

[e2]

LiveReload doesn't work with Thin on this branch.

It works when running in a separate Guard instance (using Guard groups), so I'm guessing LiveReload's EventMachine doesn't play well with fork().

This means it may be currently not possible to have "everything working" within a single Guard instance.

The only idea I have is to run Rack as a child process, and not through fork().

[e2]

Also, the build method in action.rb should catch StandardError exception, because that's what Haml errors are based on, and jekyll throws what it gets.

[e2]

The StandardError should also be caught in Builder::Rebuilder#update.

The real fix is here: https://github.com/jekyll/jekyll/issues/3289 (but it may take a long time before a change is released).

[imathis]

I'm sorry it's taking me so long to review this. I'm in the middle of working on a big release for Octopress and these are a lot of changes to digest. Do you know if these regressions due to the refactor or are they issues with being incompatible with newer versions of dependencies?

[e2]

I never got guard-jekyll-plus to work completely as intended (or as I wanted), so I'm only using this branch.

These issues are nothing new:

• using threads (and Webrick) for server interferes with Jekyll
• using fork for server interferes with LiveReload (EventMachine probably has to be forked in a special way)

I can't reasonably send more PRs (or add issues) until you merge this into master.

This PR isn't different from the current master besides the few points that I described.

Besides, LiveReload won't work if you have ignore /^_site/ in your Guardfile template (ignore works globally).

[imathis]

Looks good. On to the future :)