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

feature(frontend): Analyzer Configuration #2850

Merged

Conversation

xoscar
Copy link
Collaborator

@xoscar xoscar commented Jul 4, 2023

This PR adds the analyzer configuration changes for the frontend, including the settings page and analyzer results

Changes

  • Includes the new configuration fields as part of the settings page
  • Updates the analyzer results section with the configured error levels and weights

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

https://www.loom.com/share/f7b4e4e7fe1f46ada25ac3cda543c40e

@xoscar xoscar added the frontend label Jul 4, 2023
@xoscar xoscar self-assigned this Jul 4, 2023
@xoscar xoscar marked this pull request as ready for review July 4, 2023 18:18
@@ -32,14 +32,12 @@ const GlobalResult = ({score, minimumScore}: IProps) => {
<Percentage score={score} />
</S.GlobalScoreContainer>
</S.GlobalScoreWrapper>
{!!minimumScore && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should be always displaying this as per Olly's feedback

@jorgeepc
Copy link
Contributor

jorgeepc commented Jul 4, 2023

Hey @xoscar, amazing work! I watched your loom and have a couple of comments/questions:

  • If you disable a plugin, this is not present in the AnalyzerResults section. However, for disabled rules they are present in the AnalyzerResults section. Should we have the same behavior for disabled plugins and rules?
  • Are we applying a validation to guarantee 100% weight between all the rules?
  • If we are doing some weight validation, should we guarantee 100% weight without considering disabled/warning rules? or what is the behavior here?

@jorgeepc
Copy link
Contributor

jorgeepc commented Jul 4, 2023

A couple of findings:

  • I'm seeing an error when trying to save the analyzer resource:
    "cannot parse body: json: cannot unmarshal string into Go struct field LinterRule.spec.plugins.rules.weight of type int"
  • This component has a weird UI:
    Screenshot 2023-07-04 at 14 55 11
    Screenshot 2023-07-04 at 14 55 23

Base automatically changed from feat/analyzer-configuration-backend to feat/analyzer-configuration July 5, 2023 18:51
@xoscar xoscar merged commit 3179019 into feat/analyzer-configuration Jul 5, 2023
1 check passed
@xoscar xoscar deleted the feat/analyzer-configuration-frontend branch July 5, 2023 21:08
xoscar added a commit that referenced this pull request Jul 10, 2023
* feature(backend): Enabling Analyzer Configuration (#2842)

* feature: Enabling Analyzer Configuration

* feature: Enabling Analyzer Configuration

* cleanup and improvements

* cleanup and improvements

* improvements, upgrades and simplification

* improvements, upgrades and simplification

* adding tests for the analyzer and linter

* fixing linter runner

* updates and fixes based on PR review comments

* removing unnecessary loop

* feature(frontend): Analyzer Configuration (#2850)

* feature: Enabling Analyzer Configuration

* feature: Enabling Analyzer Configuration

* cleanup and improvements

* cleanup and improvements

* improvements, upgrades and simplification

* improvements, upgrades and simplification

* adding tests for the analyzer and linter

* fixing linter runner

* feature(frontend): Analyzer Configuration

* feature(frontend): Analyzer Configuration

* updates and fixes based on PR review comments

* clean up changes

* fix(frontend): fix analyzer score styles (#2860)

* feature(cli): Analyzer Configuration (#2873)

* remove file with conflict

* fix disabled rule logic

---------

Co-authored-by: Jorge Padilla <jorge.esteban.padilla@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants