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

Fix code scanning issues #978

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

remcohaszing
Copy link
Contributor

What does this PR do?

This resolves 30 occurrences of:

require(path.join(__dirname), './relative-path'))

By replacing them with the equivalent:

require('./relative-path')

What issues does this PR fix or reference?

30 code scanning reports on my fork, that I’m sure are also reported on the original repository, though I can’t see them.

Is it tested? How?

npm test

This resolves 30 occurrences of:

```ts
require(path.join(__dirname), './relative-path'))
```

By replacing them with the equivalent:

```
require('./relative-path')
```
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@coveralls
Copy link

Coverage Status

coverage: 84.174%. remained the same
when pulling 6800df8 on remcohaszing:fix-security-reports
into f039273 on redhat-developer:main.

@@ -786,7 +786,7 @@ describe('Auto Completion Tests', () => {
});

it('Insert required attributes at correct level', (done) => {
const schema = require(path.join(__dirname, './fixtures/testRequiredProperties.json'));
const schema = require('./fixtures/testRequiredProperties.json');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently this trades one code scanning alert for another, though the new one is not as bad as the old one.

I do agree with the new failed checks. The proper solution would be to write this at the top of the file:

import schema = require('./fixtures/testRequiredProperties.json');

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.

2 participants