Fix linting errors #32
Conversation
fixed the linting errors.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Thanks for pointing it out @pheyvaer, I changed the IDE but forgot to add the personal setup of the IDE into the .gitignore file. |
The documentation is the main reason why I added linting 😄 So yes they have to be fixed. |
argahsuknesib
requested review from
pheyvaer
and removed request for
pheyvaer
|
Running, npm run lint:tswill give no errors or warnings now. |
|
@argahsuknesib We also need to docs for the classes and their methods. Can you fix that as well? I updated the lint config for that. |
|
I got another 126 JSDoc comment warnings, I will get rid of the warnings with another commit and reassign you to review. Thanks @pheyvaer ! |
logs/aggregation.log
Outdated
There was a problem hiding this comment.
Logs don't belong in a repo, unless there is very specific reason why they should be.
There was a problem hiding this comment.
I will add them to .gitignore
package.json
Outdated
| "test": "jest --coverage", | ||
| "test:watch": "jest --watch", | ||
| "start-solid-server": "cd scripts && rm -rf data/.internal/accounts && npx community-solid-server --config ./pod/config/unsafe.json -f ./data/ --seededPodConfigJson ./pod/pod_credentials.json -w 0", | ||
| "restart-solid-server": "cd scripts && npx community-solid-server --config ./pod/config/unsafe.json -f ./data/ -w 0", | ||
| "start-solid-server-extended-lock": "cd scripts && rm -rf data/.internal/ && npx community-solid-server --config ./pod/config/extendedlock.json -f ./data/ --seededPodConfigJson ./pod/pod_credentials.json", | ||
| "test-run": "cd scripts && rm -rf data/.internal/ && npx community-solid-server --config ./pod/config/auth.json -f ./data/ --seededPodConfigJson ./pod/pod_credentials.json --workers 1", | ||
| "lint:ts": "eslint . --ext ts --report-unused-disable-directives --max-warnings 0", | ||
| "lint:ts:fix": "eslint . --ext ts --report-unused-disable-directives --max-warnings 0 --fix" | ||
| "lint:ts:fix": "eslint . --ext ts --report-unused-disable-directives --max-warnings 0 --fix", | ||
| "lint:no:warning": "eslint . --ext ts --quiet --max-warnings 0" |
There was a problem hiding this comment.
To stay in line with the other commands this should be lint:ts:no-warning.
scratch/test.ts
Outdated
There was a problem hiding this comment.
Do scratch files belong in the repo? Is this a scratch file created by your IDE maybe?
There was a problem hiding this comment.
The scratch files are created by me, in which I test some code. But they don't belong in the repo and I added them to .gitignore.
| query_registry.delete_all_queries_from_the_registry(); | ||
| }); | ||
|
|
||
| it('delete_all_queries_from_the_registry', async () => { |
There was a problem hiding this comment.
This test is with underscores in the name, but the previous one is not. This should be consist for all tests.
There was a problem hiding this comment.
ok thanks, will do.
There was a problem hiding this comment.
You can fix that in the PR where you add/change your tests.
Ok, great! I also added some minor comments of things that I noticed. |
|
I am working on the tests right now @pheyvaer, should I re-request a review when the coverage is 100%? (as the linting errors are fixed now) |
This PR is only for linting errors. So if you are changing/adding tests then this should happen in a separate branch. The linting errors for the tests that are already in this PR should be covered yes. |
| query_registry.delete_all_queries_from_the_registry(); | ||
| }); | ||
|
|
||
| it('delete_all_queries_from_the_registry', async () => { |
There was a problem hiding this comment.
You can fix that in the PR where you add/change your tests.
The pull request corresponds to the issue 30 on the repository.
I fixed the linting issues in the repository. The unit tests are still a work in progress which I will fix and add with another pull request after the linting errors are reviewed.
Running the command,
currently gives,
The warnings are documentation warnings with JSDoc which aren't important to resolve at the current moment (please correct me on this) and will be resolved in future.