-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat: added prospector code analysis tool #688
feat: added prospector code analysis tool #688
Conversation
requirements_analysis.txt
Outdated
@@ -0,0 +1 @@ | |||
prospector==1.5.1 |
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.
This can go inside here https://github.com/mindsdb/lightwood/blob/staging/tests/requirements.txt instead
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.
Alright.
That works.
.prospector.yml
Outdated
max-line-length: 120 | ||
|
||
pep8: | ||
full: true | ||
disable: | ||
- N803 # argument name should be lowercase | ||
- N806 # variable in function should be lowercase | ||
- N812 # lowercase imported as non lowercase |
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.
Could you source these from the .flake8 file? Alt jut disable this check since we already run falke8 automatically to release
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.
Alright.
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 have disabled the checks.
I think they can be added later.
Afaict the analysis finds nothing:
Which is kind of strange... as in, I'm worried it might not be analyzing all the codebase. Can you intentionally add an error somewhere to double check that it works? |
@George3d6 I just refactored the prospector to save the output in a JSON and upload it to GitHub artifiacts. You can view it in the artifacts tab for a CI run. Are you okay with this approach? |
I think this approach work, but could you link to the artefact generated by this execution? -- currently I don't see anything: https://github.com/mindsdb/lightwood/runs/4023079796?check_suite_focus=true |
Any updates on this? I feel like there may have been a few merging issues along the way :)) Feel free to open a new PR if you want. |
I’ll pull the new merges into my branch. The underlying problem is that the Github CI checks are not printing out the analysis result to the console, making it difficult to work with. |
Closing as inactive for now, might be better to just start a new PR anyway, since this branch resulted in a very odd diff. |
This pull request will fix this issue.
This pull request adds Prospector as a static code analysis tool to run an analysis each time a PR is opened through the use of GitHub actions.