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

Provide more details in dot api #850

Merged
merged 2 commits into from
Oct 3, 2019
Merged

Conversation

Mark1626
Copy link
Contributor

@Mark1626 Mark1626 commented Sep 30, 2019

Provide more details in dot api

@gaugebot gaugebot bot added the cla-signed label Sep 30, 2019
@gaugebot gaugebot bot requested a review from BugDiver September 30, 2019 19:48
lib/repl/repl.js Outdated
@@ -266,7 +266,24 @@ function displayUsageFor(name) {
return;
}
console.log();
if (e.deprecated) {
console.log(`DEPRECATED ${desc(e.deprecated)}`);
console.log();
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason for empty console.log?
If it is just to achieve a new line, consider using '\n'. If worried about os specific new lines consider using require('os').EOL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was reusing what was previously in place. EOL is a better fit as suggested.

lib/repl/repl.js Outdated
e.returns.map(e => {
console.log(`Returns: ${type(e.type)} ${(e.description.type ? desc(e.description) : e.description)}`);
})
console.log();
Copy link
Member

Choose a reason for hiding this comment

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

Empty console.log?

@Mark1626
Copy link
Contributor Author

Mark1626 commented Oct 3, 2019

@BugDiver There seems to be a lot of files not linted properly, running lint changes a lot of files. Should I raise a separate PR with all lints?

@BugDiver
Copy link
Member

BugDiver commented Oct 3, 2019

I guess that would be better.

@BugDiver BugDiver merged commit 38497d6 into getgauge:master Oct 3, 2019
@saikrishna321
Copy link
Member

@BugDiver @M4rk9696 There is a pre-commit hook that does linting.

https://github.com/getgauge/taiko/blob/master/package.json#L30-L33

@Mark1626
Copy link
Contributor Author

Mark1626 commented Oct 3, 2019

@saikrishna321 I think it is better to have a lint stage in CI as well, as a hook can be skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants