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

Add reader file as a lint check target #226

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

Add reader file as a lint check target #226

wants to merge 4 commits into from

Conversation

yjaeseok
Copy link
Contributor

@yjaeseok yjaeseok commented Jan 3, 2018

Our lint_ts command do not include
reader file. So we cannot check lint error
in reader file. So I add it.

But It is hard to fix no-relative-imports rule.
So I would like to this rule disable temporary,
Could anyone help me 'how to fix the rule issue?'

ISSUE=None

I would like to show command line usage
when user type just `$./bacard`

So I add command-line-usage package for generate
usage contents and then add implement usage in default
Our lint_ts command do not include
reader file. So we cannot check lint error
in reader file. So I add it.

But It is hard to fix no-relative-imports rule.
So I would like to this rule disable temporary,
Could anyone help me 'how to fix the rule issue?'

ISSUE=None
@@ -8,6 +8,7 @@
"match-default-export-name": false,
"max-line-length": [true, 80],
"no-banned-terms": false,
"no-relative-imports": false,
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to allow no-relative-imports.
For reference: //generator/new_parser/parser.ts.

Copy link
Member

@romandev romandev left a comment

Choose a reason for hiding this comment

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

Also, if this change works well, you will have to add this file to gulp.task('lint_ts').

@yjaeseok
Copy link
Contributor Author

yjaeseok commented Jan 3, 2018

@romandev
Oh!! I skipped gulp file!! @.@
Thank you for your review

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

Successfully merging this pull request may close these issues.

None yet

2 participants