Skip to content

Split codegen-plugins back into codegen-core and remove client-api#1091

Merged
adwsingh merged 2 commits intomainfrom
adwsingh/client-api-removal
Mar 20, 2026
Merged

Split codegen-plugins back into codegen-core and remove client-api#1091
adwsingh merged 2 commits intomainfrom
adwsingh/client-api-removal

Conversation

@adwsingh
Copy link
Contributor

Issue #, if available:

Description of changes:

The original motivation for splitting client-core into client-api was to break a dependency cycle:
codegen -> client -> framework-errors -> codegen.

In practice, this required moving all classes from client-core into client-api, leaving client-core as little more than an aggregator with a dependency on framework-errors. It also introduced an awkward limitation: client-api, which contains most of the actual client logic, can no longer depend on framework-errors.

We’re now taking a different approach. Instead of restructuring the client modules, we split codegen-plugins into a codegen-core module that contains the essential generators for types, and an internal (non-exposed) types plugin used by framework-errors. This breaks the cycle at the codegen layer without distorting the client module boundaries.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@adwsingh adwsingh force-pushed the adwsingh/client-api-removal branch from 08e5b14 to a41b47b Compare March 19, 2026 22:35
LOGGER.info("Successfully generated Java class files.");
}

private static Set<Shape> getClosure(software.amazon.smithy.model.Model model, TypeCodegenSettings settings) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, replace qualified name with import.

private static Set<Shape> getClosure(software.amazon.smithy.model.Model model, TypeCodegenSettings settings) {
Set<Shape> closure = new HashSet<>();
settings.shapes()
.stream()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, replace stream API chain with loop.

.map(model::expectShape)
.forEach(closure::add);
settings.selector()
.shapes(model)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, replace stream API chain with loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

selector().shapes() returns a Stream.

create("internal") {
compileClasspath += sourceSets["main"].output + configurations["compileClasspath"]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add something like:

+    main {
+        java.srcDir("src/internal/java")
+    }

to allow IntelliJ to identify the directory as a source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, creating a source set already does that. It does for me atleast.

Image

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried and it didn't work for me I will double check.

@adwsingh adwsingh enabled auto-merge (squash) March 20, 2026 07:51
@adwsingh adwsingh force-pushed the adwsingh/client-api-removal branch from e50fdb7 to 43f6f46 Compare March 20, 2026 08:20
@adwsingh adwsingh merged commit 91afa89 into main Mar 20, 2026
4 of 6 checks passed
@adwsingh adwsingh deleted the adwsingh/client-api-removal branch March 20, 2026 15:34
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