Add ability to specify custom SCORM API url and other minor tweaks #14
Conversation
- Added the ability to specify a custom SCORM API url via an ENV var. - Added undercased versions of ENV vars so they are usable on the command line. - Replaced use of `try` which is Rails specific with Ruby begin/rescue handler. - Updated README to reflect above changes and fix some inaccuracies.
lib/scorm_cloud/error.rb
Outdated
| @@ -6,19 +6,18 @@ class RequestError < Error | |||
| attr_reader :code, :msg, :url | |||
|
|
|||
| def initialize(doc, url) | |||
| err = doc.elements["rsp"].try(:elements).try(:[], "err") | |||
| if err | |||
| err = doc.elements["rsp"].elements["err"] | |||
There was a problem hiding this comment.
What's the motivation for this change? I don't adore try which we were doing before, but an unscoped rescue is worse, in my view. We could rescue the specific errors you'd expected from line 9, but if we are going to do that, why not try? I could easily be missing something :)
There was a problem hiding this comment.
try doesn't exist outside of rails. When I was testing this change manually using the CLI app with a a bogus course id, the above code was triggered and failed because try is undefined. I could change that to use respond_to?(:elements) instead. I think I went with the bare rescue mostly to deal with multiple tries. Thoughts on flipping over to respond_to? ?
There was a problem hiding this comment.
Oh, duh. I'm +1 on respond_to?. Consider splitting this to a few lines if needed with that solution. Clarity over brevity.
|
-1, see comment. Everything else looks good and useful. Thanks for contributing back to this. |
- Added the ability to specify a custom SCORM API url via an ENV var. - Added undercased versions of ENV vars so they are usable on the command line. - Replaced use of `try` which is Rails specific with Ruby begin/rescue handler. - Updated README to reflect above changes and fix some inaccuracies.
|
@mcwqy9 messed up my patch. new one coming. |
Added the ability to specify a custom SCORM API url via an ENV var.
Added undercased versions of ENV vars so they are usable on the
command line.
Replaced use of
trywhich is Rails specific with Ruby begin/rescuehandler.
Updated README to reflect above changes and fix some inaccuracies.