-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implement report.md formatting #148
Comments
I recommend listing also the audits weighted with 0 as they might be critical things but not possible to score... Thoughts? |
If we would inline the score of a category we could save some lines BEFORE: Performance🟢 Score: 92 AFTER: 🟢 Performance: 92 |
Remark on the "Categories Section". Let's also show how multiple resulting issues in the examples e.g. 1 error, 4 warnings Same for the issue table. BEFORE: Code style🟡 Score: 54
AFTER: Code style🟡 Score: 54
|
It is a bit "scrolly" to focus on a goal e.g. performance and have to scroll from the list of categories down to the audits and look up where it. Then start again in categories. In stead of grouping by data attribute we could take the view of our personas (we have to write them up) and try to make it easy for them to fulfil their needs. Getting better numbers in goal XYZ. I would create a persona, and put an issue in the back lock. This can be considered when we have the PR comment version |
I believe that the colors ad quite some complexity. From a product owner perspective I would try to evaluate the need for that feature with the customer. If we keep it we have to consider that not all goals are "green" at a score of 90 but also can be e.g. 60. I guess we can understand that problem better when we have implemented the assertion logic and the user defined budgets |
Hmm, there's quite a simple way to solve that, I think 🤔 In each audit's detail, we could include something like "Referenced by categories: Performance, Code style.", that way you can easily jump back to the category. I don't want to change the scope of this issue, but I can create a follow-up issue for that. What do you think?
Sure, personas need to be considered. Again, let's separate that discussion from this issue. But I can mention quickly how I think about it conceptually in relation to how the report is structured:
The color is always gonna be a bit of a simplification, because there's a limited set of values ("good", "bad" and "something in between"). But I don't think that's a problem if we also show the actual score next to it, the goal is just to add some extra visual cues. It's possible the thresholds won't be universal and we can solve that by enabling the color grading to be configurable, but I wouldn't complicate it too early until we get some more definitive customer feedback. |
Sorry, didn't scroll up all the way and I missed some of the earlier comments 🤦
Yes, I didn't mention this explicitly (added it now), but I would simply list every audit/group the category references, regardless of weight.
That also looks good for categories, in my opinion. But I think we should be consistent with audits as well (a result should ideally always look the same, e.g. in my proposal it's always
Yes, that's how it will be formatted 🙂 It's actually already handled by the ESLint plugin (implementation here - you can see it uses helper functions from This actually makes the formatter's logic quite simple, it basically just has to do |
Yes let's create something and use it a bit to feel it in real life. |
Implemented the changes for the report.md file formatting. Added the following sections: Overview, Categories, Audits, About. Closes #148
As user whose trying out Code PushUp for the first time, I want an easy to read (i.e. not raw JSON) report after I've succesfully ran the CLI for the first time (based on getting started docs).
Based on discussion in #93
General notes
Math.round(score * 100)
(0-100, only integers)<details>
(including<summary>
) must use HTML formatting, Markdown won't be renderedscore < 0.5
=> red0.5 <= score < 0.9
=> yellowscore >= 0.9
=> greenStructure
error
=> 🚨,warning
=>info
=> ℹ️ (or nothing?)7
if only start line,7-10
if both (different) start and end lines,-
if no linesnew Date().toString()
n/a
)Example
Follows below line...
Code PushUp report
🏷️ Categories
Performance
🟢 Score: 92
key
props in iterators/collection literals (ESLint) - 1 errorCode style
🟡 Score: 54
let
orconst
instead ofvar
(ESLint) - 0🛡️ Audits
Largest Contentful Paint (Lighthouse)
🟨 1.5 s (score: 75)
📖 Docs
Disallow missing
key
props in iterators/collection literals (ESLint)🟥 1 error (score: 0)
Issues
src/components/TodoList.jsx
ESLint 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.js
src/hooks/useTodos.js
src/hooks/useTodos.js
ESLint rule object-shorthand. 📖 Docs
Require
let
orconst
instead 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.0
The following plugins were run:
0.1.0
0.1.0
The text was updated successfully, but these errors were encountered: