-
Notifications
You must be signed in to change notification settings - Fork 77
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
Replace Grunt with gulp #12
Conversation
@@ -1,20 +1,17 @@ | |||
exports.config = { | |||
allScriptsTimeout: 11000, | |||
allScriptsTimeout: 11000, | |||
seleniumPort: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Their protractor config doc says set it to null
if you want to let selenium server find its own port. I see it works without it too by defaulting to 4444, but not sure if it's gonna pick another port if 4444 is taken. This is what I use in all my configs. I can remove it here, if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope it's fine, makes sense to leave it in even if it's the default just to be sure. In the past I haven't had an issue either way.
Can you change the tabspacing to 4 spaces per tab (soft tabs) in the Gulpfile? and also follow the convention like this: var x = function () { notice the spacing between function and () and () and { |
I can't seem to reply under your last comment anymore, but I've fixed the function calls to insert that extra space before I've also replaced 2 space indentation with 4 (cried while I doing it, though :) ) |
Thanks @demisx I know a lot of people like the 2 spaces, I like the readability of 4 spaces and I'm used to it from my previous experience. Everything else looks very good though! Thanks so much for pushing this through :) |
Replace Grunt builder with gulp.js. Tried to preserve the name of the tasks as much as I could, so they simply map from
grunt <taskName>
togulp <taskName>
. Please review.Fixes #10.