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

fix(browsers): add PhantomJS as default browser #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Merott
Copy link

@Merott Merott commented May 15, 2014

If no browser is specified by the user, the karma task will not run,
getting stuck at Starting Karma Server.

Make PhantomJS the default browser to use.

If no browser is specified by the user, the karma task will not run,
getting stuck at `Starting Karma Server`.

Make PhantomJS the default browser to use.
@joshdmiller
Copy link
Member

That's true. I meant to add a warning message and a default failure without a browsers key being present. Karma isn't saying anything because I turned up the logging level - it was polluting the Warlock output with a lot of useless junk.

The consequence of specifying a default is that, because it's an array, it cannot be overwritten by the user - it can only be appended to. So if a user specifies Firefox it will run Karma on both browsers.

I'd like to be able to override the merging functionality, but I can't figure out how to signal that in a JSON-friendly way. e.g. this is ugly:

{
  "globs": {
    "source": {
      "js": [
        "$overwrite",
        "my/replacement/glob"
      ]
    }
  },
  "tasks": {
    "karma": {
      "single-run": {
        "$overwrite": true,
        "..."
      }
    }
  }
}

And this is so detached as to be confusing:

{
  "globs": {
    "source": {
      "js": [
        "my/replacement/glob"
      ]
    }
  },
  "config": {
    "no-merge": [
      "globs.source.js"
    ]
  }
}

Anyway, I don't think it's necessarily a problem to run on PhantomJS too, but I wanted to call it out as a reality. If we're good with that, I'll happily merge. Thanks!

@Merott
Copy link
Author

Merott commented May 16, 2014

I see!

Actually, personally I'd prefer to be able to override the default and would find it inconvenient not to be able to. So if you ask me, don't merge this PR, until it's possible to overwrite configs :)

I noticed you mentioned the lack of functionality to completely overwrite a config in the Wiki as well. I think I may have a cleaner solution for replacing configs while still being able to merge. That's a topic of its own, belonging to a repository of its own, so, here is my proposed solution.

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.

2 participants