Skip to content
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: complexity checker plugin #169

Merged
merged 61 commits into from
Nov 14, 2024
Merged

Conversation

kharrop
Copy link
Contributor

@kharrop kharrop commented Sep 24, 2024

Addresses #162.

Covers the following scenarios:

  • Asset nodes
  • Asset node complexity based on type
  • Nested assets within templates (and nested templates)
  • View nodes
  • View node complexity based on type
  • State expressions
  • Model - get expressions
  • Model - evaluations

This plugin will read out the complexity score per content provided and supports info, warn, and error Diagnostic severities. For more verbose logs, -v trace will display a breakdown of what is contributing to the score.

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
📦 Published PR as canary version: 0.8.2--canary.169.3894

Try this version out locally by upgrading relevant packages to 0.8.2--canary.169.3894

Version

Published prerelease version: 0.9.0-next.2

Changelog

Release Notes

Add options to LSPAssetsPlugin to load from TSManifest via module import (#171)

Allow for loading XLRs to LSP via module imports. Add explicit (optional for now) toggle for distinguishing between module and manifest loading.


🚀 Enhancement

🐛 Bug Fix

Authors: 6

@kharrop kharrop linked an issue Sep 24, 2024 that may be closed by this pull request
4 tasks
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.25%. Comparing base (f82434d) to head (01e0096).
Report is 62 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
- Coverage   73.59%   73.25%   -0.34%     
==========================================
  Files          87       85       -2     
  Lines       13049    12876     -173     
  Branches     1228     1201      -27     
==========================================
- Hits         9603     9432     -171     
+ Misses       3426     3423       -3     
- Partials       20       21       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

*/
maxWarningLevel?: number;
/** If set, maps complexity based on asset or view type */
assetComplexity?: Record<string, number>;
Copy link
Member

@sugarmanz sugarmanz Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to consider accepting any as a key so we can assign weights for assets that are registered with more than just a type, as those might have more complexity than the base asset is, i.e. if { type: 'asset', role: 'enhanced' } accepts another asset slot than just { type: 'asset' }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this callout - agreed that including other slots than just type to measure asset complexity would provide additional data points, but at this time we're going to keep it simple just to get this out. I've added in TODO comments so that we don't forget about this.

@kharrop
Copy link
Contributor Author

kharrop commented Sep 26, 2024

/canary

2 similar comments
@kharrop
Copy link
Contributor Author

kharrop commented Sep 26, 2024

/canary

@kharrop
Copy link
Contributor Author

kharrop commented Oct 1, 2024

/canary

@kharrop kharrop force-pushed the feature/measure-content-complexity branch from d451d1d to 27b1aa2 Compare October 1, 2024 19:47
@kharrop
Copy link
Contributor Author

kharrop commented Oct 30, 2024

/canary

2 similar comments
@kharrop
Copy link
Contributor Author

kharrop commented Oct 30, 2024

/canary

@kharrop
Copy link
Contributor Author

kharrop commented Nov 6, 2024

/canary

@kharrop
Copy link
Contributor Author

kharrop commented Nov 8, 2024

/canary

this.config = config;
this.typeCount = {};
this.contentScore = 0;
this.range = { start: 0, end: 0 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this.range is assigned a value but I'm not sure I see it ever being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was being used for FlowNode but since we're not actually doing anything with that (only with Flow State Node), we can remove it?

@kharrop
Copy link
Contributor Author

kharrop commented Nov 12, 2024

/canary

1 similar comment
@kharrop
Copy link
Contributor Author

kharrop commented Nov 13, 2024

/canary

@kharrop kharrop force-pushed the feature/measure-content-complexity branch from 58c17e2 to f781093 Compare November 14, 2024 15:51
KetanReddy
KetanReddy previously approved these changes Nov 14, 2024
@kharrop kharrop merged commit 9af842c into main Nov 14, 2024
7 of 8 checks passed
@kharrop kharrop deleted the feature/measure-content-complexity branch November 14, 2024 20:57
@intuit-svc intuit-svc mentioned this pull request Dec 3, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build Out Code Complexity Metrics
7 participants