-
Notifications
You must be signed in to change notification settings - Fork 26
feat: app data conversion reports #2424
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
Conversation
…ing-app-ui into feat/app-build-manifest
…ernational/parenting-app-ui into feat/app-build-manifest
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.
Very nice. I can see how there's quite a lot to be done in order to exhaustively capture some of the useful metrics in these reports, including some refactoring of the app code. But it's pleasing to see the reports generated in this current version.
Previously a specific
Sheets Summary
was included in console outputs. This is now included in reports with the hopes that it might be more obvious/actionable if appearing in git diff when making content PRs. If still useful to keep in the console then it can still be logged (as could any other report) - just not currently to reduce the amount displayed
I think nice to have this removed and the output tidier.
@@ -0,0 +1,34 @@ | |||
// TODO - move to generic location (possibly object-utils once #2423 merged) |
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.
#2423 should be ready to merge now
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.
Thanks, refactored with 8e4b098
Unsurprisingly I did find the method already existed in the codebase (but within file-utils
which wasn't very obvious), so have merged the code to store in shared object-utils
A relevant issue for follow-ups: #1979 |
…ing-app-ui into feat/app-build-manifest
PR Checklist
Description
This PR lays groundwork for a more comprehensive QA and Optimisation system to make per-deployment recommendations (or future automation). Specifically it adds a Reports script that generates reports based on data processed during content sync.
An initial
TemplateSummary
report has been included which iterates through templates and enumerates the number of times each component and action type is used. In the future this will be used to help identify components that can be removed from deployment bundles.Report summaries are stored within the deployment folder, in both json and markdown formats for easier interpretation
This is part of Hosting RFC milestone
Review Notes
The fastest way to run the subset of the sync process that generates reports is via:
See output in deployment
reports
folder. Can also see example output belowAuthor Notes
It would be useful to discuss and consider what other reports might be most useful to authors, and the best way to format output data. E.g. we could also include large assets, asset usage, unused templates, invalid actions etc. Some of these are already captured during sync (errors/warning thrown), but more could be added and moved to reports.
For the current report there doesn't appear to be any use of dynamic data references, however should be noted that the reports generate from static data only. If dynamic data is used, e.g. a row component type defined as
@local.use_this_component_type
, then only the dynamic text will show in the report and not the intended value. This may mean that some reports aren't as accurate if referring to dynamic data. It might be possible to work around this for cases where naming conventions are used, e.g. assets referenced in data_lists but with a name_asset
The report outputs are populated to the deployment content repo folder. If a deployment doesn't wish to include these reports then they can add to gitignore, e.g.
.gitignore
Previously a specific
Sheets Summary
was included in console outputs. This is now included in reports with the hopes that it might be more obvious/actionable if appearing in git diff when making content PRs. If still useful to keep in the console then it can still be logged (as could any other report) - just not currently to reduce the amount displayedDev Notes
Tests can be run by the individual scripts referenced in the spec files
One of the major challenges moving forwards when it comes to optimisation based on reports, is how to understand potential side-effects. E.g. the authors may not use the
button
component directly, but could reference another component that itself imports it (peer dependency). Similarly for actions, understanding which feature registers which action/action namespace is vital. So before moving ahead with generated optimisations more work will be required to separate out features that register actions and/or component namespaces, and expose a common declaration syntax for those modules that the reporters can understand.However as this will take some time to do, it might make sense in the short term to at least try to start using the reports to generate outputs that can either be informative or actionable by authors (e.g. identifying poorly optimised/duplicate/hanging assets, templates or data lists)
Future TODOs
(should organise post-merge)
Template Summary Report
Potential Reports
Git Issues
Closes #
Screenshots/Videos
CLI outputs report location during sheet processing

Example output from debug generated report
Summary
Actions
Components
Flows By Type