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

Requested additions for :server_mount option #138

Merged
merged 4 commits into from
Aug 6, 2013

Conversation

adamstrickland
Copy link
Contributor

  • Added section to README for integrating with JasmineRails
  • Added CLI support for :server_mount option (as --mount or -m)
  • If neither :server_mount and :jasmine_url option is not provided, will auto-build URL using /specs if JasmineRails is found and /jasmine if it is not

@@ -25,7 +25,7 @@ class Jasmine < Guard
server: :auto,
server_env: ENV['RAILS_ENV'] || 'development',
server_timeout: 60,
server_mount: '/jasmine',
server_mount: '/jasmine', # set here for documnetation puprposes; actually determiend at runtime by presence (or lack thereof) of the JasmineRails constant
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typos

@@ -148,6 +153,7 @@ def spec(*paths)
runner_options[:spec_dir] = options.spec_dir || (File.exists?(File.join('spec', 'javascripts')) ? File.join('spec', 'javascripts') : 'spec')
runner_options[:server] = options.server.to_sym == :auto ? ::Guard::Jasmine::Server.detect_server(runner_options[:spec_dir]) : options.server.to_sym
runner_options[:jasmine_url] = options.url || "http://localhost:#{ runner_options[:port] }#{ options.server.to_sym == :jasmine_gem ? '/' : '/jasmine' }"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this line also need to interpolate options.mount instead of hard-coding /jasmine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point... I had intentionally not added that in, figuring that to leave the default mount point to be /jasmine in the interest of minimum disruption, but it would be more internally consistent. Anyone with add'l thoughts on the matter?

@netzpirat
Copy link
Contributor

Hi, I'm on holiday and won't be able to have a look at this before the first week of August.

@netzpirat netzpirat merged commit d0da317 into guard:master Aug 6, 2013
@netzpirat
Copy link
Contributor

Merged with some minor changes in the specs to set the JasmineRails module with stub_const. Thanks a lot!

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.

None yet

4 participants