Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use entire namespace so exception constant is resolved #243

Merged
merged 3 commits into from
Jan 17, 2016

Conversation

simeonwillbanks
Copy link
Contributor

Problem

#241 introduced a custom exception, but it used the exception outside the HTML::Pipeline namespace.

~/Code/html-pipeline master
❯ irb -I lib
[1] pry(main)> require 'html/pipeline'
=> true
[2] pry(main)> filter = HTML::Pipeline::MarkdownFilter.new("Hi **world**!")
NameError: uninitialized constant MissingDependencyError
from /Users/simeonwillbanks/Code/html-pipeline/lib/html/pipeline/markdown_filter.rb:4:in `rescue in <top (required)>'
[3] pry(main)> require "github/markdown"
LoadError: cannot load such file -- github/markdown
from /usr/local/var/rbenv/versions/2.3.0/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'

Solution

Resolve the exception constant by using the full namespace.

~/Code/html-pipeline add-namespace-to-missing-deps-raise
❯ git diff
diff --git a/lib/html/pipeline/markdown_filter.rb b/lib/html/pipeline/markdown_filter.rb
index 94a820a..b16dfc8 100644
--- a/lib/html/pipeline/markdown_filter.rb
+++ b/lib/html/pipeline/markdown_filter.rb
@@ -1,7 +1,7 @@
 begin
   require "github/markdown"
 rescue LoadError => _
-  raise MissingDependencyError, "Missing dependency 'github-markdown' for MarkdownFilter. See README.md for details."
+  raise HTML::Pipeline::MissingDependencyError, "Missing dependency 'github-markdown' for MarkdownFilter. See README.md for detai
 end

 module HTML

~/Code/html-pipeline add-namespace-to-missing-deps-raise*
❯ irb -I lib
[1] pry(main)> require 'html/pipeline'
=> true
[2] pry(main)> filter = HTML::Pipeline::MarkdownFilter.new("Hi **world**!")
HTML::Pipeline::MissingDependencyError: Missing dependency 'github-markdown' for MarkdownFilter. See README.md for details.
from /Users/simeonwillbanks/Code/html-pipeline/lib/html/pipeline/markdown_filter.rb:4:in `rescue in <top (required)>'

/cc @parkr @jch

@simeonwillbanks
Copy link
Contributor Author

@parkr Thanks for the original Pull Request! Sorry we missed this in code review. 😊

@parkr
Copy link
Contributor

parkr commented Jan 15, 2016

@simeonwillbanks Thanks for catching this! Apologies for the dud.

@jch
Copy link
Contributor

jch commented Jan 15, 2016

@simeonwillbanks 🙇 thanks for catching my mistake. @parkr no need to apologize, I should've been more thorough with the review.

@simeonwillbanks would you be open to releasing a patch release?

@simeonwillbanks
Copy link
Contributor Author

@jch prepping release...

simeonwillbanks added a commit that referenced this pull request Jan 17, 2016
Use entire namespace so exception constant is resolved :shipit:
@simeonwillbanks simeonwillbanks merged commit f92b3db into master Jan 17, 2016
@simeonwillbanks simeonwillbanks deleted the add-namespace-to-missing-deps-raise branch January 17, 2016 22:04
@simeonwillbanks
Copy link
Contributor Author

@parkr
Copy link
Contributor

parkr commented Jan 17, 2016

Thank you, @simeonwillbanks! 🎉

@jch
Copy link
Contributor

jch commented Jan 18, 2016

🎉

On Sun, Jan 17, 2016 at 2:47 PM, Parker Moore [email protected]
wrote:

Thank you, @simeonwillbanks https://github.com/simeonwillbanks! [image:
🎉]


Reply to this email directly or view it on GitHub
#243 (comment).

Jerry Cheung
GitHub https://github.com/jch

@simeonwillbanks
Copy link
Contributor Author

😉

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

Successfully merging this pull request may close these issues.

3 participants