Skip to content

Conversation

@JakobHaastertSB
Copy link

Package dependencies can now be shown. In case the OSGI-INF directory is not available, it can be generated from the jar.

Copy link
Member

@NiklasRentzCAU NiklasRentzCAU left a comment

Choose a reason for hiding this comment

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

Due to all the formatting changes, this PR is very hard to read and the actual changes are not visible in some of the bigger files. Please re-do this PR without all the formatting changes so I can actually review this.
Some of the changes I did notice and left some individual comments in the changed lines. We can discuss how to continue with this PR in our next meeting.

Comment on lines +8 to +12
<buildCommand>
<name>org.eclipse.xtext.ui.shared.xtextBuilder</name>
<arguments>
</arguments>
</buildCommand>
Copy link
Member

Choose a reason for hiding this comment

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

Are these XtextBuilder and -nature now necessary due to the Maven Mojo you added or what do you need this for?

Comment on lines 134 to 151
private void connectBundlesViaPackages() {
project.getPackages().forEach(p -> connectBundlesViaSinglePackageDependency(p));
}

private void connectBundlesViaSinglePackageDependency(Package inputPackage) {
if (inputPackage.getConnectingPackageExportDependencyBundles().isEmpty()) {
LOGGER.log(System.Logger.Level.DEBUG, inputPackage.getName() + " has no exported package");
return;
}

Bundle exportedBundle = inputPackage.getConnectingPackageExportDependencyBundles().get(0);
inputPackage.getConnectingPackageImportDependencyBundles().forEach(bundle -> {
bundle.getConnectedPackageDependencyBundles().add(exportedBundle);
getCommonProductForBundles(bundle, exportedBundle)
.forEach(product -> addOrUpdatePackageDependencyEdgeLabel(bundle, exportedBundle, product));
});

}
Copy link
Member

Choose a reason for hiding this comment

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

Let us talk about what you want to achieve here in our meeting, to me it seems that this re-wiring of connections is something that SPViz could do for you instead, probably by extending the DSL's functionality.

@NiklasRentzCAU
Copy link
Member

Very nice, this is much easier to understand now. PR looks good in general, I just left a few minor (unresolved) comments, once they are resolved/answered I am happy to merge this.

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.

2 participants