Filters Manage Dependencies #80

simeonwillbanks · 2013-08-25T20:25:45Z

Fixes #77

Add dependency management test cases for each Filter with a
dependency
Add assert_dependency_management_error custom assertion which
asserts a custom exception and message are raised if a dependency
is missing
Move all Filter dependencies to Gemfile :test block for
test cases and CI
Implement TestingDependency helper to abstract unloading and
loading Gemfile :test block gems when asserting dependency
management errors
Implement MissingDependencyException custom exception with
MESSAGE constant as a format string, so each Filter raises a
uniform exception
Add begin..rescue..end blocks around each Filter require
statement to raise a MissingDependencyException when a gem can
not be loaded
Update README.md detailing new dependency management with listing
of Filter gem dependencies
Add gemspec post install message to inform users their apps must
bundle Filter dependencies

* Add dependency management test cases for each `Filter` with a dependency * Add `assert_dependency_management_error` custom assertion which asserts a custom exception and message are raised if a dependency is missing * Move all `Filter` dependencies to `Gemfile` `:test` block for test cases and CI * Implement `TestingDependency` helper to abstract unloading and loading `Gemfile` `:test` block gems when asserting dependency management errors * Implement `MissingDependencyException` custom exception with `MESSAGE` constant as a format string, so each `Filter` raises a uniform exception * Add `begin..rescue..end` blocks around each `Filter` `require` statement to raise a `MissingDependencyException` when a gem can not be loaded * Update README.md detailing new dependency management with listing of `Filter` gem dependencies * Add gemspec post install message to inform users their apps must bundle `Filter` dependencies

simeonwillbanks · 2013-08-25T20:27:51Z

Notes/Questions:

Error messages do not have versions.
- Hard coding versions meant they were repeated in filters, tests and
  the Gemfile; this wasn't maintainable.
- Dynamically setting versions seemed overkill. e.g. Bundler.load.specs.find{ |s| s.name == "rinku" }
Should activesupport be a full dependency?
- ActiveSupport::XmlMini_Nokogiri
  is required by
  HTML::Pipeline.
  It was added
  here.
  Should we remove the require statement or add acivesupport as a full
  dependency? Maybe I'm missing something. 😄

simeonwillbanks · 2013-08-26T15:05:32Z

lib/html/pipeline/emoji_filter.rb

@@ -1,4 +1,9 @@
-require 'emoji'
+begin
+  require "gemoji"


gemoji.rb is just a stub for emoji.rb. Changing this back to require "emoji" will eliminate a require. Thoughts?

I'd leave it alone in case the gem ever decides to load more things in gemoji.rb.

Good point.

jch · 2013-08-27T07:43:38Z

Thanks for the pull! I've left comments inline.

* Copy edit README.md * In gemspec, link README.md dependencies and update post install message * Rename test helper to `assert_dependency` * Remove `TestingDependency` in favor of stubbing `Kernal#require`

…in #to_hash is only used by #assert_equal_html

simeonwillbanks · 2013-08-28T18:06:49Z

@jch You're welcome!

The latest Travis build was terminated. It hung on $ sudo apt-get install -qq libicu-dev. Can you please restart the build? Thanks.

jch · 2013-08-28T18:44:26Z

re: double gem. Sounds good.

Job restarted.

On Wed, Aug 28, 2013 at 11:06 AM, Simeon Willbanks <notifications@github.com

wrote:

@jch https://github.com/jch You're welcome!

The latest Travis buildhttps://travis-ci.org/jch/html-pipeline/jobs/10719183was terminated. It hung on $
sudo apt-get install -qq libicu-dev. Can you please restart the job?
Thanks.

[image: travis-ci-html-pipeline]https://f.cloud.github.com/assets/164506/1044162/809911d8-100c-11e3-9784-290c910e916d.png

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-23435121
.

-Jerry
@whatcodecraves http://twitter.com/whatcodecraves
github http://github.com/jch

jch · 2013-09-26T02:16:48Z

Just an update, but I pinged some other GitHubbers for feedback. I think the code here is fine, but wanted to get a 2nd pair of 👀 on it before I released it.

simeonwillbanks · 2013-09-26T02:24:24Z

Sounds good!

jch · 2013-10-22T18:16:50Z

README.md

+* `SyntaxHighlightFilter` - `github-linguist`
+* `TextileFilter` - `RedCloth`
+
+_Note:_ See [Gemfile](https://github.com/jch/html-pipeline/blob/master/Gemfile) `:test` block for version requirements.


GitHub markdown supports relative links now. So you can just do /Gemfile.

…-management

…ttle

…ge to STDERR.

simeonwillbanks · 2013-10-23T03:52:34Z

@jch This Pull Request has many twists and turns. Before you merge, maybe you want to rebase? If so, for your convenience, here is a list of pertinent commit messages.

Add begin..rescue..end blocks around Filter require gem statements. If a dependency is missing, exit with a 1 status and write help message to STDERR.
Move all Filter dependencies to Gemfile :test block for test cases and CI.
Add gemspec post install message to inform users their apps must bundle Filter dependencies.
Update README.md detailing new dependency management with listing of Filter gem dependencies.

jch · 2013-10-23T15:33:04Z

No need to rebase, just fetch and merge master into your branch. I like
having all the twists and turns in the history because it matches up with
the pull request discussion.

On Tuesday, October 22, 2013, Simeon Willbanks wrote:

@jch https://github.com/jch This Pull Request has many twists and
turns. Before you merge, maybe you want to rebase? If so, for your
convenience, here is a list of pertinent commit messages.

Add begin..rescue..end blocks around Filter require gem statements.
If a dependency is missing, exit with a 1 status and write help message to
STDERR.

Move all Filter dependencies to Gemfile :test block for test cases
and CI.

Add gemspec post install message to inform users their apps must
bundle Filter dependencies.

Update README.md detailing new dependency management with listing of
Filter gem dependencies.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/80#issuecomment-26878827
.

-Jerry
@whatcodecraves http://twitter.com/whatcodecraves
github http://github.com/jch

simeonwillbanks · 2013-10-23T19:58:48Z

Cool. It's good to go. Last night, I fetched and merged master.

Filters Manage Dependencies

jch · 2013-10-23T20:02:24Z

I'll cut a new release after I try this out on a few applications first.

simeonwillbanks · 2013-10-23T20:10:41Z

Sounds good!

parkr · 2016-01-14T18:15:28Z

@simeonwillbanks @jch The move to rescue LoadError; abort has broken the ability to handle lack of dependencies in the client applications. See: gjtorikian/jekyll-html-pipeline#7 (comment).

simeonwillbanks reviewed Aug 26, 2013
View reviewed changes

Simeon F. Willbanks added 3 commits August 27, 2013 19:19

Address Pull Request comments:

c25c890

* Copy edit README.md * In gemspec, link README.md dependencies and update post install message * Rename test helper to `assert_dependency` * Remove `TestingDependency` in favor of stubbing `Kernal#require`

Textile Filter does require RedCloth

f0983dc

Move ActiveSupport::XmlMini_Nokogiri to test helper, since its mixed …

f9c6d06

…in #to_hash is only used by #assert_equal_html

Merge upstream/master; activesupport is a full dependency

e042c3e

jch reviewed Oct 22, 2013
View reviewed changes

Simeon F. Willbanks added 5 commits October 22, 2013 19:36

Relative link to Gemfile

0471de4

Merge remote-tracking branch 'upstream/master' into simple-dependency…

3801fd5

…-management

Don't test dependency management since its testing require and is bri…

154ef94

…ttle

No need for MissingDependencyException

6ff25ad

If a dependency is missing, exit with a 1 status and write help messa…

a0acb69

…ge to STDERR.

jch added a commit that referenced this pull request Oct 23, 2013

Merge pull request #80 from simeonwillbanks/simple-dependency-management

42f013f

Filters Manage Dependencies

jch merged commit 42f013f into gjtorikian:master Oct 23, 2013

simeonwillbanks mentioned this pull request Feb 4, 2014

EmailReplyParser is undefined #107

Closed

jch mentioned this pull request Feb 25, 2015

Separate gems for versioning external dependencies #182

Closed

parkr mentioned this pull request Jan 14, 2016

rescue LoadError; abort flow for missing dependencies breaks client applications #240

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filters Manage Dependencies #80

Filters Manage Dependencies #80

simeonwillbanks commented Aug 25, 2013

simeonwillbanks commented Aug 25, 2013

simeonwillbanks Aug 26, 2013

jch Aug 27, 2013

simeonwillbanks Aug 27, 2013

jch commented Aug 27, 2013

simeonwillbanks commented Aug 28, 2013

jch commented Aug 28, 2013

jch commented Sep 26, 2013

simeonwillbanks commented Sep 26, 2013

jch Oct 22, 2013

simeonwillbanks Oct 22, 2013

simeonwillbanks commented Oct 23, 2013

jch commented Oct 23, 2013

simeonwillbanks commented Oct 23, 2013

jch commented Oct 23, 2013

simeonwillbanks commented Oct 23, 2013

parkr commented Jan 14, 2016

Filters Manage Dependencies #80

Filters Manage Dependencies #80

Conversation

simeonwillbanks commented Aug 25, 2013

simeonwillbanks commented Aug 25, 2013

simeonwillbanks Aug 26, 2013

Choose a reason for hiding this comment

jch Aug 27, 2013

Choose a reason for hiding this comment

simeonwillbanks Aug 27, 2013

Choose a reason for hiding this comment

jch commented Aug 27, 2013

simeonwillbanks commented Aug 28, 2013

jch commented Aug 28, 2013

jch commented Sep 26, 2013

simeonwillbanks commented Sep 26, 2013

jch Oct 22, 2013

Choose a reason for hiding this comment

simeonwillbanks Oct 22, 2013

Choose a reason for hiding this comment

simeonwillbanks commented Oct 23, 2013

jch commented Oct 23, 2013

simeonwillbanks commented Oct 23, 2013

jch commented Oct 23, 2013

simeonwillbanks commented Oct 23, 2013

parkr commented Jan 14, 2016