-
Notifications
You must be signed in to change notification settings - Fork 184
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
Load package.json
on init-linter
#4363
Conversation
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
packages/jsts/tests/analysis/fixtures/package-json/rule-package-json/package.json
Outdated
Show resolved
Hide resolved
packages/jsts/tests/analysis/fixtures/package-json/rule-package-json/rules.js
Outdated
Show resolved
Hide resolved
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
packages/jsts/src/dependencies/package-json/project-package-json.ts
Outdated
Show resolved
Hide resolved
…on.ts Co-authored-by: Yassin Kammoun <[email protected]>
…on.ts Co-authored-by: Yassin Kammoun <[email protected]>
Co-authored-by: Yassin Kammoun <[email protected]>
Co-authored-by: Yassin Kammoun <[email protected]>
Co-authored-by: Yassin Kammoun <[email protected]>
Co-authored-by: Yassin Kammoun <[email protected]>
make baseDir mandatory
always ignore .scannerwork folder
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.
LGTM! Great job!
} | ||
|
||
async walkDirectory(dir: string, ignoredPatterns: Minimatch[]) { | ||
const files = await fs.readdir(dir, { withFileTypes: true }); |
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.
why do you need to await
the result here, if function is async
?
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.
because otherwise files is a promise and would not be able to iterate
continue; // is ignored pattern | ||
} | ||
if (file.isDirectory()) { | ||
await this.walkDirectory(filename, ignoredPatterns); |
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.
why await
?
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.
not needed but to keep order of execution
protected List<String> getExcludedPaths() { | ||
var configuration = this.context.config(); | ||
var excludedPatterns = Arrays.asList(JavaScriptPlugin.EXCLUSIONS_DEFAULT_VALUE); | ||
var jsExcludedPatterns = configuration.get(JavaScriptPlugin.JS_EXCLUSIONS_KEY).isPresent() |
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.
you don't need to do this, if you call getStringArray
it will return []
if the property is not defined
|
||
protected List<String> getExcludedPaths() { | ||
var configuration = this.context.config(); | ||
var excludedPatterns = Arrays.asList(JavaScriptPlugin.EXCLUSIONS_DEFAULT_VALUE); |
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.
we don't need this variable
@@ -126,4 +132,18 @@ protected boolean shouldAnalyzeWithProgram(List<InputFile> inputFiles) { | |||
LOG.debug("Will use AnalysisWithProgram"); | |||
return true; | |||
} | |||
|
|||
protected List<String> getExcludedPaths() { |
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.
we should share this logic with PathAssesor
, by creating utility class with static function, otherwise we will forget to keep the two places in sync
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 wanted to do this, but had doubts about the how-to
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.
@vdiez pls see the comment about sharing the exclusion logic
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
No description provided.