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

Use inheritance-aggregator model #2589

Merged

Conversation

jianghaolu
Copy link
Contributor

No description provided.

@@ -38,7 +38,4 @@
</dependencies>
</dependencyManagement>

<modules>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use inheritance over multi-module for a simpler module management.

As the number of child SDKs grow, each of them is independent, with the only commonality being the parent pom. The parent pom isn't aware of the children either so child projects can change without modifying this section.

This file serves as the universal parent for dependencies, plugins, etc. The only time to modify this file is when we decide to upgrade a dependency version or alter a build process.

<description>This package bundles all the SDKs in a multi-module for build/CI purposes.</description>
<url>https://github.com/Azure/azure-sdk-for-java</url>

<modules>
Copy link
Contributor Author

@jianghaolu jianghaolu Nov 10, 2018

Choose a reason for hiding this comment

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

This file serves as an aggregator for all the projects. To build all of child modules, run

mvn -f pom.client.build.xml compile

This file will not be consumed by any human. This file can be modified at any time without notifying any group, or testing any existing libraries. When a new SDK is added, it will be added to this list for it to be included in the build process.

There can be many pom files like this one for all kinds of purposes.

@AutorestCI
Copy link
Contributor

Build output:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section is needed so that when you build this pom file Maven will know where to look for the parent. Without this section, Maven will not look for child module's parent poms.

@mikeharder
Copy link
Member

What is the benefit of moving the <modules> section to a second file pom.client.build.xml, rather than just keeping it in pom.client.xml? I'm fine with taking this change, but it seems to add unnecessary complexity by splitting the same information into two files rather than one.

@jianghaolu
Copy link
Contributor Author

This way we keep pom.client.xml as simple as possible for dependency and plugin management. Nobody ever needs to touch this file except for dependency management, reducing the errors possibly introduced to this file which everyone using Azure will appreciate.

This PR, though looks redundant and introduces complexities, actually changes the project structure from a multi-module model to a inheritance-aggregator model. Multi-module projects are designed for big projects that are split apart for easier management. The components (almost always) share the same version, release cycle, and has strong inter-component dependencies. I don't think our data plane libraries across services make for such a case.

@mikeharder mikeharder merged commit 7014512 into Azure:mikeharder/template Nov 13, 2018
mikeharder pushed a commit that referenced this pull request Nov 13, 2018
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.

3 participants