Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Only warn about an unsupported TypeScript version once #347

Merged
merged 6 commits into from
Aug 9, 2017

Conversation

j-f1
Copy link
Contributor

@j-f1 j-f1 commented Aug 8, 2017

It gets annoying when you’re trying to lint a project with lots of files. Fixes #348.

@eslintbot
Copy link

Thanks for the pull request, @j-f1! I took a look to make sure it's ready for merging and found some changes are needed:

  • The commit summary needs to begin with a tag (such as Fix: or Update:). Please check out our guide for how to properly format your commit summary and update it on this pull request.

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@soda0289
Copy link
Member

soda0289 commented Aug 8, 2017

I think we can move the warning text outside the parse() that should eliminate the use of a global variable

…lint#348)

It gets annoying when you’re trying to lint a project with lots of files.
@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@j-f1
Copy link
Contributor Author

j-f1 commented Aug 8, 2017

@soda0289 Done!

@eslintbot
Copy link

LGTM

@soda0289
Copy link
Member

soda0289 commented Aug 8, 2017

@j-f1
Thanks for making the change but on a second look I noticed we need to have access to the extra.log() and dont think this should be outside parse(). That was added since we did not want to write logs when other projects that use this parser. We still need to have the ability for users of the library to overwrite the logger.

I think we will have to use the global variable. We should move the warning text to its own function and contain the logic of checking if it has been called to there. Sorry for the confusion and thanks for the PR.

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

parser.js Outdated
@@ -99,11 +100,14 @@ function parse(code, options) {
*/
if (typeof options.loggerFn === "function") {
extra.log = options.loggerFn;
} else if (options.loggerFn === false) {
/** */ // eslint-disable-line valid-jsdoc
Copy link
Member

@JamesHenry JamesHenry Aug 9, 2017

Choose a reason for hiding this comment

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

This is a bit odd 😄 Why not put the // eslint-disable-line valid-jsdoc on the line of the function you are disabling? Alternatively you could use Function.prototype as the value of the noop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I don’t add the JSDoc comment, it errors at line 98 instead. I’ll switch to Function.prototype.

@eslintbot
Copy link

LGTM

@JamesHenry
Copy link
Member

Thanks a lot for this @j-f1!

@JamesHenry JamesHenry merged commit 81e20c0 into eslint:master Aug 9, 2017
@j-f1 j-f1 deleted the patch-1 branch August 9, 2017 12:27
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