-
Notifications
You must be signed in to change notification settings - Fork 8
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: give analyze the ability to display browsable results #94
Conversation
I'm not sure if this is what was wanted but this is my attempt of giving something that stores the results in a readable form |
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.
Hey @Poiuy7312 , can you please be more descriptive on what changes you have made on this PR? It would be helpful for your team if you could provide a description of what you have made that closes the issue #30 .
Hi @Poiuy7312 can you give an example of what the command-line looks like? |
Thanks @Poiuy7312 this is an awesome feature! |
Hello @Poiuy7312 can you confirm that this is only for the |
Also @tuduun, @laurennevill, and/or @jnormile can you review this and provide feedback? |
Yes this is only for analyze
…On Wed, Nov 1, 2023, 16:44 Gregory M. Kapfhammer ***@***.***> wrote:
Hello @Poiuy7312 <https://github.com/Poiuy7312> can you confirm that this
is only for the analyze sub-command?
—
Reply to this email directly, view it on GitHub
<#94 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A2ZWPMBLUFSQ5VSBTYWWGWDYCKYDTAVCNFSM6AAAAAA6OEC3OGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBZGY2TEMRVGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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 think this looks good. Thank you for including a descriptive PR with screenshots and examples. Great work!
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.
One really nitpicky thing - I would say that in main.py, on line 390, I would order the error codes to ignore numerically, so it goes # noq: PLR0912, PLR0913, PLR0915
.
Something I just noticed as well - make sure you update to master as well |
Just as a note, merging master into your branch still hasn't fixed the downgrading of |
@Poiuy7312 I was going to try and resolve the conflicts between the master main.py and the main.py here, but I'm not sure what some of the resolutions should be to maintain functionality. It's only three conflicts, so it shouldn't take long to resolve when you get the chance. |
closes #30
This gives analyze the sub commands --markdown-storage/-r and --display for storing the results in a markdown file and displaying it as a browsable frogmouth display when using --display
Ex
chasten analyze chasten --config $PWD/.chasten/ --debug-level ERROR --debug-dest CONSOLE --search-path . -r . --display
Display example: