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

[AGD-2060] Linting API #11673

Merged
merged 4 commits into from
May 11, 2021
Merged

[AGD-2060] Linting API #11673

merged 4 commits into from
May 11, 2021

Conversation

SHKnudsen
Copy link
Contributor

Purpose

This PR adds new linting capabilities to Dynamo. The PR consist of Linting APIs which includes a Linter manager that handles subscription to the active linter and manages all the linter issues in the graph furthermore it adds a new LinterExtensionBase which extensions can implement to make a linter extension, the other part of the PR is the linter view extensions which adds a UI to the linter manager where all the issues are displayed.

Currently, the LinterViewExtension is disabled in the manifest file and the LinterExtensionBase has been made internal.

Linter Manager PR - #11606
Linter ViewExtension PR - #11634

LinterViewUpdatedUi_v2_x175

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@BogdanZavu @saintentropy @mjkkirschner @nate-peters

FYIs

SHKnudsen added 2 commits May 7, 2021 10:26
* Initial commit

* Move event subscription to extension base class + add property changed filter to rules

* comment updates

* Update LinterExtensionBase.cs

* Update InputNodesNotAllowedRule.cs

* Add Deactivate method to linterExtensionBase + LinterManager tests

* comment updates

* Dispose LinterManager

* comment updates

* Serialize linter manager to dyn file

* clear ruleEvaluationResults on workspace change

* Remove test extension

* Handle GraphRuleEvaluationResults nodeIds changes

* Update SerializationConverters.cs

* Fix failing serialization test

* comment updates

* revert changes to test file
* Initial commit

* Move event subscription to extension base class + add property changed filter to rules

* comment updates

* Update LinterExtensionBase.cs

* Update InputNodesNotAllowedRule.cs

* Add Deactivate method to linterExtensionBase + LinterManager tests

* comment updates

* Initial commit

* Add issues count to WorkspaceSaving

* add scrollviewer

* Dispose LinterManager

* Make concrete issues internal

* summaries on IRuleIssue

* comment updates

* Update severity code icon

* Serialize linter manager to dyn file

* clear ruleEvaluationResults on workspace change

* Remove test extension

* Show names instead of GUIDs

* add help doc

* clean up

* Handle GraphRuleEvaluationResults nodeIds changes

* Update LintingViewExtension.csproj

* Update LintingViewExtension.csproj

* Update SerializationConverters.cs

* Fix failing serialization test

* comment updates

* revert changes to test file

* fix available linters binding

* add rules back in test ext

* comment updates

* remove output path

* Update LinterManagerTests.cs

* comment updates

* hide LinterViewExtension and LinterExtensionBase

* Update LinterViewModel.cs
@BogdanZavu BogdanZavu force-pushed the AGD-2060-Linting-API branch from cd60aef to 6add9ce Compare May 7, 2021 14:39
@SHKnudsen SHKnudsen mentioned this pull request May 11, 2021
8 tasks
* Linter manager (#11606)

* Initial commit

* Move event subscription to extension base class + add property changed filter to rules

* comment updates

* Update LinterExtensionBase.cs

* Update InputNodesNotAllowedRule.cs

* Add Deactivate method to linterExtensionBase + LinterManager tests

* comment updates

* Dispose LinterManager

* comment updates

* Serialize linter manager to dyn file

* clear ruleEvaluationResults on workspace change

* Remove test extension

* Handle GraphRuleEvaluationResults nodeIds changes

* Update SerializationConverters.cs

* Fix failing serialization test

* comment updates

* revert changes to test file

* Linter ViewExtension (#11634)

* Initial commit

* Move event subscription to extension base class + add property changed filter to rules

* comment updates

* Update LinterExtensionBase.cs

* Update InputNodesNotAllowedRule.cs

* Add Deactivate method to linterExtensionBase + LinterManager tests

* comment updates

* Initial commit

* Add issues count to WorkspaceSaving

* add scrollviewer

* Dispose LinterManager

* Make concrete issues internal

* summaries on IRuleIssue

* comment updates

* Update severity code icon

* Serialize linter manager to dyn file

* clear ruleEvaluationResults on workspace change

* Remove test extension

* Show names instead of GUIDs

* add help doc

* clean up

* Handle GraphRuleEvaluationResults nodeIds changes

* Update LintingViewExtension.csproj

* Update LintingViewExtension.csproj

* Update SerializationConverters.cs

* Fix failing serialization test

* comment updates

* revert changes to test file

* fix available linters binding

* add rules back in test ext

* comment updates

* remove output path

* Update LinterManagerTests.cs

* comment updates

* hide LinterViewExtension and LinterExtensionBase

* Update LinterViewModel.cs

* if statements

* remove Prism

* clean up
{
public class LintingViewExtension : ViewExtensionBase
{
private const string EXTENSION_NAME = "Graph Status";
Copy link
Member

@mjkkirschner mjkkirschner May 11, 2021

Choose a reason for hiding this comment

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

hmm @BogdanZavu @SHKnudsen - maybe put this in a resx file?

Copy link
Member

Choose a reason for hiding this comment

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

never mind for now.

@BogdanZavu BogdanZavu merged commit ca18e9d into master May 11, 2021
@BogdanZavu BogdanZavu deleted the AGD-2060-Linting-API branch May 11, 2021 19:01
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.

4 participants