refactor: create parent pom #776
Conversation
| <version>0.0.1-SNAPSHOT</version> | ||
| <relativePath>sorald-parent</relativePath> | ||
| </parent> | ||
|
|
||
| <groupId>se.kth.castor</groupId> | ||
| <artifactId>sorald</artifactId> |
There was a problem hiding this comment.
Technically speaking, we should also change sorald to sorald-sonar or something along those lines. But I am not sure how do we handle releases then. For example, what actions to take on maven to instruct users that sorald is now sorald-sonar?
There was a problem hiding this comment.
I wouldn't touch the artifact name of a released artifact. This can destroy some infrastructure without a real benefit. And sorald-sonar is somewhat incorrect because it's not only sonar as analyzer.
There was a problem hiding this comment.
This can destroy some infrastructure without a real benefit.
Yes, I was reluctant too.
And sorald-sonar is somewhat incorrect because it's not only sonar as analyzer.
Oh, I thought sorald-parent would eventually contain all the SPI stuff while the sorald package would be one of its service.
There was a problem hiding this comment.
No, the next step is to create sorald-api and sorald-sonar. The parent is only for aggregation and maven reasons there. Otherwise, we would need to deploy all new artifacts(API and sonar module) directly to maven central before we can proceed.
There was a problem hiding this comment.
The parent is only for aggregation and maven reasons there.
I am not sure I understand this. Could you please elaborate?
There was a problem hiding this comment.
Sure,
In this diagram, we see on the left side the current monolithic structure. We have a single component/module, Sorald. This includes sonar-lint as dependency. On the right side we see the new structure. We have multiple modules namely sorald-api, sorald-qodana, sorald-sonar and sorald.
Sorald API is the analyzer API implemented by sorald-qodana and sorald-sonar. This API is reusable for other static analyzers and contains the interfaces like StaticAnalyzer or SoraldAbstractProcessor. Sorald is the current Sorald module without the API interfaces and sonar package. Sorald has Sorald-API and Sorald-Sonar as dependency declared in its pom. Both are currently local modules. And all of them share a lot of infrastructure in their pom, like spotless config or JUnit dependency declarations. We want to avoid declaring multiple JUnit dependencies, with worst case multiple versions. Adding a parent containing all this infrastructure simplifies this.
The new folder structure looks like this:
We have a submodule for each analyzer and the api. The sorald src folder stays in its place. The structure is more or less like spoon, spoon-decompiler and spoon-pom.
Does this help you?
There was a problem hiding this comment.
Yes, this is very helpful and indeed this becomes very similar to Spoon. I just had one follow up question from the diagram. Shouldn't we decouple Sorald and Sorald API as well? In other words, Sorald should not have a direct dependency on Sorald API. As far as I understand, Sorald will eventually be a CLI and depending upon the argument it takes, it can either invoke sorald-sonar or sorald qodana.
There was a problem hiding this comment.
Sorald should not have a direct dependency on Sorald API.
Sorald needs the Sorald API for service loading. We load with SPI all StaticAnalyzers, and we can only load the class with a reference to it. We could decouple in more modules and event split the analyzer API more fine granular, but each split is a time investment that needs to be worth it.
As far as I understand, Sorald will eventually be a CLI and depending upon the argument it takes, it can either invoke sorald-sonar or sorald qodana.
This sounds like refactoring the CLI from Sorald to a new artifact. You are more or less talking about the red marked area I added.
These are new artifacts dependent on Sorald. I would start with creating the structure from the first diagram and build the lower part you mentioned in the future, after we integrated Qodana successfully. There can exist way more Sorald dependent artifacts and this is not limited to the two.
There was a problem hiding this comment.
Sorald needs the Sorald API for service loading.
Oh right. I forgot that we would need references to the services defined.
There can exist way more Sorald dependent artifacts and this is not limited to the two.
Of course, let's take it step-by-step. The initial diagram and directory structure you proposed above seems nice. We should work towards it.
I will merge this PR now.
There was a problem hiding this comment.
@khaes-kth do check this thread out. @MartinWitt is working to split Sorald into multiple modules so that integration of more static analyzers becomes easier.
This PR creates a parent pom, containing all dependencies we need in all submodules. With this parent-pom we could choose to adopt a layout similar to spoon and move way less files.