Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

display disableChecks option in extra flags error message #3964

Merged
merged 2 commits into from
Jan 13, 2017

Conversation

igniteram
Copy link
Contributor

With reference to this recent SO question , it would be good to display the --disableChecks option in the error message thrown in case users have their own custom flags set in the cli.

Users are not aware of this option until they check the changelog, it would help if this is displayed along with the error message thrown in the console itself.

@cnishina Since you worked on this, any thoughts?

Copy link
Contributor

@cnishina cnishina left a comment

Choose a reason for hiding this comment

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

This is a good improvement. I've seen a few of these issues pop up and have been directing them to the changelog.

@@ -181,7 +181,7 @@ if (!argv.disableChecks) {
});

if (unknownKeys.length > 0) {
throw new Error('Found extra flags: ' + unknownKeys.join(', '));
throw new Error('Found extra flags: ' + unknownKeys.join(', ') + ', please use --disableChecks flag to disable the custom flag check!');
Copy link
Contributor

Choose a reason for hiding this comment

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

The wording could be improved.
please use --disableChecks flag to disable the Protractor CLI flag checks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, CircleCI does not like your typescript formatting. Please run npm run format before submitting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, noticed that let me change & format it!

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

Successfully merging this pull request may close these issues.

4 participants