Skip to content
This repository has been archived by the owner on Dec 22, 2018. It is now read-only.

#14 - All specs video path #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jsgervais
Copy link

#14 - Allow specifying other name than protractor-specs.mov in case, when one video file is produced.

Added a new option called allSpecsVideoName which can be either a string or a function. It is combined with the baseDirectory option to create a full path.

jeansebastieng added 2 commits August 23, 2017 11:35
… case, when one video file is produced.

Added a new option called allSpecsVideoName which can be either a string or a function.   It is combined with the baseDirectory option to create a full path.
@tomyam1
Copy link
Owner

tomyam1 commented Aug 30, 2017

Hi Jean
Thanks for PR!
Some issues before this can be merged:

  • There are some linting issues in the new code, e.g. space in if ( typeof....
  • Add validation for the options "You must also define baseDirectory path" - this should be validated with JOI.
  • allSpecsVideoName should be changed to allSpecsVideoPath for consistency with singleVideoPath.
  • Please don't change the version yet. I'll update it in a separate commit.

I think that's it.

@@ -1,6 +1,6 @@
{
"name": "protractor-video-reporter",
"version": "0.3.0",
"version": "0.3.1",
Copy link

Choose a reason for hiding this comment

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

As stated, please do not change version numbers in a regular PR. Version number should be changed when we do a release

@yeikel
Copy link

yeikel commented Apr 16, 2018

@tomyam1 Is this PR dead? Last activity was way back

@@ -56,6 +56,20 @@ For example, you can do:
result.fullName + '.mov';
}


* `allSpecsVideoName`: (function):

Choose a reason for hiding this comment

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

Since both singleVideo === true and baseDirectory required by default, maybe this should be called defaultVideoName?


For example, you can do:

allSpecsVideoName: function (result) {

Choose a reason for hiding this comment

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

The result argument here isn't actually passed to the user function in the implementation. Fix impl or remove the false doc.

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

Successfully merging this pull request may close these issues.

4 participants