search

devonfw / sonar-devon4j-plugin

Plugin for SonarQube to validate devonfw architecture
Apache License 2.0
10 stars 16 forks source link

Feature/97 custom configuration for architecture #106

Closed adprus closed 3 years ago

adprus commented 3 years ago

97: added class ArchitectureJsonReader and fields in Architecture.class read from architecture.json

hohwille commented 3 years ago

BTW: Did you read my comment/suggestion in issue #97:

So my suggestion would be to define a new class DevonArchitecturePackage as a kind of "wrapper" for Devon4jPackage that offers the same API methods and delegates them to an internal Devon4jPackage instance. Could you create a PR for this as a first step?

adprus commented 3 years ago

yes, I have. I have talked about this with Lmar. First we wanted to know if we are in a Devon project, that's why I have implemented this ArchitectureJsonReader. And now I'm working on using this ConfigurationMapper instead.

adprus commented 3 years ago

Of course, I will do this DevonArchitecturePackage as a first step :)

hohwille commented 3 years ago

First we wanted to know if we are in a Devon project, that's why I have implemented this ArchitectureJsonReader. And now I'm working on using this ConfigurationMapper instead.

Sure, also fine and we need the configuration mapping anyhow. But just as a suggestion: I would simply remove the devon4j-basic dependency and the usage of Devon4jPackage entirely from this plugin. Instead just add the defaults for packages configuration in the model. E.g. via lazy static getter in Architecture like this

public static getPackages(Architecture architecture) {
  Packages packages = null;
  if (architecture != null) {
    packages = architecture.getPackages();
  }
  if (packages == null) {
    packages = Packages.getDefault();
  }
  return packages;
}

Why static and not lazy regular getter? IMHO because then the JSON mapping works properly bidirectional and you do not write the defaults back to JSON if they have not been present in the parsed JSON.

So Packages.getDefault() provides the defaults such that the behaviour using regex groups behaves just like Devon4jPackage did before.

hohwille commented 3 years ago

I merged your changes online on github so before you continue you should first pull your feature branch to avoid any conflicts.