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.

Sign up for GitHub

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

Jump to bottom

Sort the type names in the list so the code gen is deterministic. #4138

Merged
merged 4 commits into from
May 27, 2022
Merged

Sort the type names in the list so the code gen is deterministic. #4138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d71326d
Sort the type names in the list so the code gen is deterministic.
eboudrant May 25, 2022
5c6aa67
update fixtures
eboudrant May 25, 2022
6d68f30
update java codegen too
eboudrant May 25, 2022
600772f
update java fixtures
eboudrant May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ler/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/java/file/SchemaBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class SchemaBuilder(

private fun typesFieldSpec(): FieldSpec {
val allTypenames = interfaces.map { it.name } + objects.map { it.name } + unions.map { it.name }
val initilizer = allTypenames.map {
val initilizer = allTypenames.sortedBy { it }.map {
CodeBlock.of("$T.type", context.resolver.resolveSchemaType(it))
}.toListInitializerCodeblock(withNewLines = true)

Expand Down
2 changes: 1 addition & 1 deletion ...r/src/main/kotlin/com/apollographql/apollo3/compiler/codegen/kotlin/file/SchemaBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class SchemaBuilder(
builder.add("listOf(\n")
builder.indent()
builder.add(
allTypenames.map {
allTypenames.sortedBy { it }.map {
CodeBlock.of("%T.type", context.resolver.resolveSchemaType(it))
}.joinToString(", ")
)
Expand Down
8 changes: 4 additions & 4 deletions ...est/graphql/com/example/__schema/java/operationBased/__schema/type/__Schema.java.expected
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions ...test/graphql/com/example/__schema/kotlin/responseBased/__schema/type/__Schema.kt.expected
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions apollo-compiler/src/test/graphql/com/example/measurements
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// If you updated the codegen and test fixtures, you should commit this file too.

Test: Total LOC:
aggregate-all 189197
aggregate-kotlin-responseBased 48615
aggregate-all 189514
aggregate-kotlin-responseBased 48932
aggregate-kotlin-operationBased 34136
aggregate-kotlin-compat 35553
aggregate-java-operationBased 70893
Expand Down Expand Up @@ -240,6 +240,7 @@ kotlin-responseBased-subscriptions
kotlin-responseBased-enums_as_sealed 340
kotlin-responseBased-operation_id_generator 325
kotlin-responseBased-merged_include 323
kotlin-responseBased-big_query 317
kotlin-responseBased-enum_field 316
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why this line was added?

Copy link
Contributor

Choose a reason for hiding this comment

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

2 PRs were merged in parallel:

So in the end 4132 was merged without the new test that you just added. So thank you :)

FWIW, this file is informative only. It's used to keep track of the size of codegen. It's handy to have it commited because it allows to easily bisect if there is a regression but we might as well put it in .gitignore

kotlin-responseBased-case_sensitive_enum 315
kotlin-responseBased-nonnull 309
Expand Down