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(analyser): add config builder to extend the analyser config #730

Merged

Conversation

ekarademir
Copy link
Contributor

Closes #720

Adds a builder to create a CompareConfig that can be used for creating Compare class.

Changes

  • Add a CompareConfigBuilder class that starts from an empty configuration and able to build up configuration as we go.
  • As a final step user can call the build method to get the resulting configuration.

Flags

Screenshots or Video

Related Issues

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

@ekarademir ekarademir requested a review from a team October 10, 2023 17:05
@ekarademir ekarademir changed the title Ertugrul/i720/extendable analyser feat(analyser): add config builder to extend the analyser config Oct 10, 2023
dselman and others added 6 commits October 10, 2023 18:10
* feat(decoratormanager) add validate method

Signed-off-by: Ertugrul Karademir <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
* feat(map): add property based flag

Signed-off-by: Jonathan Casey <[email protected]>

* fix typo

Signed-off-by: Jonathan Casey <[email protected]>

---------

Signed-off-by: Jonathan Casey <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
* feat(map): add jsdoc for enableMapType option

Signed-off-by: Jonathan Casey <[email protected]>

* chore(*): update changelog

Signed-off-by: Jonathan Casey <[email protected]>

---------

Signed-off-by: Jonathan Casey <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
@ekarademir ekarademir force-pushed the ertugrul/i720/extendable_analyser branch from 6e98c14 to 200acfc Compare October 10, 2023 17:11
Copy link
Member

@mttrbrts mttrbrts left a comment

Choose a reason for hiding this comment

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

A well designed extension, thanks @ekarademir!

Do you understand why we have the linting/whitespace changes? Is there a build step that we should be automating, or are these changes made by your editor's configuration?

packages/concerto-analysis/src/compare-config.ts Outdated Show resolved Hide resolved
packages/concerto-analysis/src/compare-config.ts Outdated Show resolved Hide resolved
@ekarademir
Copy link
Contributor Author

Do you understand why we have the linting/whitespace changes? Is there a build step that we should be automating, or are these changes made by your editor's configuration?

What generated those differences was to run the lint command within the package with --fix option with auto formatting turned off. We can definitely add an automated step. I can look into that.

@jonathan-casey jonathan-casey requested a review from a team October 11, 2023 12:17
@jonathan-casey
Copy link
Member

Nice work @ekarademir

@ekarademir ekarademir force-pushed the ertugrul/i720/extendable_analyser branch from d536e59 to 8441a77 Compare October 13, 2023 14:40
Signed-off-by: Ertugrul Karademir <[email protected]>
Signed-off-by: Ertugrul Karademir <[email protected]>
@ekarademir ekarademir force-pushed the ertugrul/i720/extendable_analyser branch from 8441a77 to f562818 Compare October 13, 2023 14:43
@mttrbrts mttrbrts merged commit 1faa5d7 into accordproject:main Oct 20, 2023
11 checks passed
@ekarademir ekarademir deleted the ertugrul/i720/extendable_analyser branch October 20, 2023 16:56
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.

Provide an API to extend the default rules of the analyser
4 participants