-
Notifications
You must be signed in to change notification settings - Fork 46
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
Feature/remove post fix #172
base: master
Are you sure you want to change the base?
Feature/remove post fix #172
Conversation
@@ -11,7 +11,7 @@ export default class Differencify { | |||
} | |||
this.configuration = sanitiseGlobalConfiguration(conf); | |||
this.browser = null; | |||
this.testId = 0; | |||
conf.removePostFix === true ? this.testId = undefined : this.testId = 0 |
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.
Missing a semicolon
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.
Although you don't need this logic here at all.
@@ -57,7 +57,9 @@ export default class Differencify { | |||
} | |||
|
|||
init(config) { | |||
this.testId += 1; | |||
if (this.testId !== undefined) { |
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.
I would rather do this here
this.testId = this.configuration ? undefined : this.testId + 1;
configuration.testId = testId; | ||
if (testId !== undefined) { | ||
configuration.testId = testId; | ||
} |
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.
also add
configuration.removePostFix = checkProperty(conf, 'removePostFix', 'boolean')
? conf.removePostFix
: globalConfig.saveCurrentImage;
To sanitiseGlobalConfiguration
function.
and
removePostFix: false,
to defaultConfigs.js
Hi @Stanimir-Georgiev, thanks for your contribution. I added couple of comments. Would you please also write some unit tests too? |
#157
Allows passing config object property, that removes the index posfix