You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is source file in the issues linkeable to the line of code?
It could be, yes. Similar as for the commit, it all depends on if we can parse their remote origin from Git (GitHub/Gitlab/... - could be tricky for self-hosted repos). Then we can link to both the commit and file lines in that commit.
Plugin
Audits
Duration
this info could be collapsed with other more detailed infos to keep the report small?
Could be. I think the 1st table with category scores should be the most prominent. The next two tables are some other general info, which is good to have somewhere, but usually not the most interesting part. We could put it at the end, I suppose (most probably won't scroll that far) 🤔
The Issue table will not scale very well when we have longer descriptions from lighthouse or the nx-validators
Depends how long the message texts are. For lint errors, they're usually one short sentence. The text in cells is wrapped automatically, so I think it'll look OK even if there are 2 or 3 lines used. We could also have a character limit (replace overflowed text with ...) if it turns out some messages are really long.
Also we should see how the vertical space grows when we have a full report
Yes, the scaling will be interesting. I'd be a little concerned with the amount of audits (having hundreds of ESLint rules is fairly realistic, for example), and most concerned with the issues (some rules will have hundreds or thousands of errors in large codebases). The issues are always collapsed, so visually that shouldn't be a problem. But the file size could get out of hand. A possible solution would be to generate multiple Markdown files (e.g. one for each audit), but lets cross that bridge when we get there (might also be worth considering HTML output at that point 😅).
@BioPhoton I've "sexed up" the Markdown report a bit.
Mainly I added emojis. For category and group scores, I'm using coloured circles, and coloured squares for audits. I strongly disagree with 100=green and <100=red (highly misleading for non-binary scores, e.g. 90 shouldn't be red), so propose we use a simplified version of the colour grading used in portal - 90-100=green, 50-89=yellow (orange is too similar to red) and 0-49=red.
I moved the audit result above the description. And I moved the two less interesting tables to a final "About" section with some extra text.
I created a separate issue for the implementation that we can refine: #148
I separated the git web URLs, since they're quite complex to implement and not a must have, in my opinion: #149
To be clear, the goal for me is for a CLI user to get "something meaningful" without having to connect to the portal. The report.json is too raw, but report.md should be readable. It's still gonna "look like Markdown" though (obviously), if we want something really stylized then that we should pick a different format (report.html or even report.pdf), but I think that's overkill for now.
The main problem I want to address is that I don't think we can confidently ship the current report.md (screenshot below), which isn't logically structured, misses crucial information (e.g. audit results) and has broken formatting in general.
As far posting it as a comment in PR, we could support an abbreviated version, I guess (only title and category scores table). But honestly, I don't think that workflow is well thought out. PR comments are a very different feature for me, because they're only valuable if they show you what changed.
Activity
kristihoskova commentedon Oct 11, 2023
Need more refining
matejchalk commentedon Oct 18, 2023
@BioPhoton I've put together a small example of how the Markdown report could be formatted. What do you think?
Code PushUp report
0.1.0Categories
Performance
Score: 92
keyprops in iterators/collection literals (ESLint) - 1 errorCode style
Score: 54
letorconstinstead ofvar(ESLint) - 0Audits
Largest Contentful Paint (Lighthouse)
Link to docs
1.5 s (score: 75)
Disallow missing
keyprops in iterators/collection literals (ESLint)ESLint rule jsx-key, from react plugin.
Link to docs
1 error (score: 0)
Issues
src/components/TodoList.jsxRequire or disallow method and property shorthand syntax for object literals (ESLint)
ESLint rule object-shorthand.
Link to docs
4 warnings (score: 0)
Issues
src/hooks/useTodos.jssrc/hooks/useTodos.jssrc/hooks/useTodos.jsRequire
letorconstinstead ofvar(ESLint)ESLint rule no-var.
Link to docs
0 (score: 100)
BioPhoton commentedon Oct 18, 2023
Like it a lot!
Is source file in the issues linkeable to the line of code?
BioPhoton commentedon Oct 18, 2023
this info could be collapsed with other more detailed infos to keep the report small?
BioPhoton commentedon Oct 18, 2023
Issues
The Issue table will not scale very well when we have longer descriptions from lighthouse or the nx-validators
BioPhoton commentedon Oct 18, 2023
Also we should see how the vertical space grows when we have a full report
matejchalk commentedon Oct 18, 2023
It could be, yes. Similar as for the commit, it all depends on if we can parse their remote origin from Git (GitHub/Gitlab/... - could be tricky for self-hosted repos). Then we can link to both the commit and file lines in that commit.
Could be. I think the 1st table with category scores should be the most prominent. The next two tables are some other general info, which is good to have somewhere, but usually not the most interesting part. We could put it at the end, I suppose (most probably won't scroll that far) 🤔
Depends how long the message texts are. For lint errors, they're usually one short sentence. The text in cells is wrapped automatically, so I think it'll look OK even if there are 2 or 3 lines used. We could also have a character limit (replace overflowed text with
...) if it turns out some messages are really long.Yes, the scaling will be interesting. I'd be a little concerned with the amount of audits (having hundreds of ESLint rules is fairly realistic, for example), and most concerned with the issues (some rules will have hundreds or thousands of errors in large codebases). The issues are always collapsed, so visually that shouldn't be a problem. But the file size could get out of hand. A possible solution would be to generate multiple Markdown files (e.g. one for each audit), but lets cross that bridge when we get there (might also be worth considering HTML output at that point 😅).
BioPhoton commentedon Oct 18, 2023
The markdown will also be used in PR comments...
Having some smart handling for big sets of data would be interesting.
Also a bundle size estimation would help to discuss this.
[-]Improve MD Report [/-][+]Improve MD Report[/+]matejchalk commentedon Oct 23, 2023
@BioPhoton I've "sexed up" the Markdown report a bit.
Mainly I added emojis. For category and group scores, I'm using coloured circles, and coloured squares for audits. I strongly disagree with 100=green and <100=red (highly misleading for non-binary scores, e.g. 90 shouldn't be red), so propose we use a simplified version of the colour grading used in portal - 90-100=green, 50-89=yellow (orange is too similar to red) and 0-49=red.
I moved the audit result above the description. And I moved the two less interesting tables to a final "About" section with some extra text.
Code PushUp report
🏷️ Categories
Performance
🟢 Score: 92
keyprops in iterators/collection literals (ESLint) - 1 errorCode style
🟡 Score: 54
letorconstinstead ofvar(ESLint) - 0🛡️ Audits
Largest Contentful Paint (Lighthouse)
🟨 1.5 s (score: 75)
📖 Docs
Disallow missing
keyprops in iterators/collection literals (ESLint)🟥 1 error (score: 0)
Issues
src/components/TodoList.jsxESLint rule jsx-key, from react plugin.
📖 Docs
Require or disallow method and property shorthand syntax for object literals (ESLint)
🟥 4 warnings (score: 0)
Issues
src/hooks/useTodos.jssrc/hooks/useTodos.jssrc/hooks/useTodos.jsESLint rule object-shorthand.
📖 Docs
Require
letorconstinstead ofvar(ESLint)🟩 0 (score: 100)
ESLint rule no-var.
📖 Docs
About
Report was created by Code PushUp on Tue Oct 24 2023 09:38:51 GMT+0200 (Central European Summer Time).
0.1.0The following plugins were run:
matejchalk commentedon Oct 25, 2023
@BioPhoton @vmasek
I created a separate issue for the implementation that we can refine: #148
I separated the git web URLs, since they're quite complex to implement and not a must have, in my opinion: #149
To be clear, the goal for me is for a CLI user to get "something meaningful" without having to connect to the portal. The
report.jsonis too raw, butreport.mdshould be readable. It's still gonna "look like Markdown" though (obviously), if we want something really stylized then that we should pick a different format (report.htmlor evenreport.pdf), but I think that's overkill for now.The main problem I want to address is that I don't think we can confidently ship the current
report.md(screenshot below), which isn't logically structured, misses crucial information (e.g. audit results) and has broken formatting in general.As far posting it as a comment in PR, we could support an abbreviated version, I guess (only title and category scores table). But honestly, I don't think that workflow is well thought out. PR comments are a very different feature for me, because they're only valuable if they show you what changed.
[-]Improve MD Report[/-][+]Design: Improve MD Report[/+]matejchalk commentedon Oct 27, 2023
Closing this since the implementation is already refined: #148