Skip to content

Merge documentation of same inline modules#4008

Merged
drganjoo merged 2 commits intomainfrom
fahadzub/merge-inline-doc
Feb 14, 2025
Merged

Merge documentation of same inline modules#4008
drganjoo merged 2 commits intomainfrom
fahadzub/merge-inline-doc

Conversation

@drganjoo
Copy link
Contributor

@drganjoo drganjoo commented Feb 11, 2025

When two inline modules with the same name are generated from different parts of the codebase, their documentation should be merged. However, other metadata must match exactly, as it is an error for one part of the codebase to define an inline module with pub visibility while another defines it with pub(crate) visibility.

This PR enables documentation merging while maintaining strict validation of other metadata fields.

Currently, the following sample model fails to generate code because both SomeList and member are generated in the same inline module but with different doc comments:

@documentation("Outer constraint has some documentation")
@length(max: 3)
list SomeList {
    @length(max: 8000)
    member: String
}

@drganjoo drganjoo force-pushed the fahadzub/merge-inline-doc branch 2 times, most recently from 207b69c to 420527e Compare February 11, 2025 18:17
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • Server Test (ignoring whitespace)
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/merge-inline-doc branch from 420527e to c66fa96 Compare February 11, 2025 19:02
@drganjoo drganjoo marked this pull request as ready for review February 11, 2025 19:03
@drganjoo drganjoo requested a review from a team as a code owner February 11, 2025 19:03
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • Server Test (ignoring whitespace)
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/merge-inline-doc branch from c66fa96 to b6bec8f Compare February 12, 2025 15:06
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • Server Test (ignoring whitespace)
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/merge-inline-doc branch from b6bec8f to b7bb68e Compare February 12, 2025 16:13
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/merge-inline-doc branch from b7bb68e to 4eb4331 Compare February 12, 2025 16:48
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo enabled auto-merge February 12, 2025 18:27
@drganjoo drganjoo disabled auto-merge February 12, 2025 18:28
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@drganjoo drganjoo added this pull request to the merge queue Feb 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 14, 2025
@drganjoo drganjoo added this pull request to the merge queue Feb 14, 2025
Merged via the queue into main with commit 66692b3 Feb 14, 2025
44 of 45 checks passed
@drganjoo drganjoo deleted the fahadzub/merge-inline-doc branch February 14, 2025 16:31
drganjoo added a commit that referenced this pull request Feb 14, 2025
When two inline modules with the same name are generated from different
parts of the codebase, their documentation should be merged. However,
other metadata must match exactly, as it is an error for one part of the
codebase to define an inline module with pub visibility while another
defines it with pub(crate) visibility.

This PR enables documentation merging while maintaining strict
validation of other metadata fields.

Currently, the following sample model fails to generate code because
both `SomeList` and `member` are generated in the same inline module but
with different doc comments:

```smithy
@documentation("Outer constraint has some documentation")
@Length(max: 3)
list SomeList {
    @Length(max: 8000)
    member: String
}
```

Co-authored-by: Fahad Zubair <fahadzub@amazon.com>
@drganjoo drganjoo mentioned this pull request Feb 20, 2025
aws-sdk-rust-ci pushed a commit that referenced this pull request Feb 20, 2025
This PR adds the CHANGELOG entries that were missed in these two earlier
PRs:

#4010    
#4008
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