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

chore: clean up unnecessary TODOs in smithy-swift #1217

Closed
2 tasks
Tracked by #710
dayaffe opened this issue Oct 31, 2023 · 5 comments
Closed
2 tasks
Tracked by #710

chore: clean up unnecessary TODOs in smithy-swift #1217

dayaffe opened this issue Oct 31, 2023 · 5 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@dayaffe
Copy link
Collaborator

dayaffe commented Oct 31, 2023

Describe the feature

The following TODOs can be removed as they are no longer necessary:

https://github.com/smithy-lang/smithy-swift/blob/53fbde27f536f4287edfd4ff08bb5ebe8dc75169/design/DESIGN.md

  • let's take a look at this one as a group

// TODO: this file will change post AWS Service config design most likely.

  • This file does not seem to have needed any changes as it has not been touched in 2 years

/// TODO: simplify logger creation

  • Logger creation seems to work well and does not need to be simplified further

// TODO:: fix indentation when writeInline retains indent

  • Indentation issues don't seem pervasive & comment is 2 years old

TODO:: When https://github.com/apple/swift-numerics supports Integer conforming to Real protocol, we need to
change [UInt8] to Complex. Apple's work is being tracked in apple/swift-numerics#5

// TODO create resource type

  • the same TODO exists in smithy-python as well
  • let's take a look at this one as a group

// TODO: MUST FIX BEFORE SHIPPING. check to see if new synthetic input or output shapes conflict with any other shapes in the model by walking the model and fail code generation

  • comment is 2 years old and we haven't run into any issues with conflicting shapes during codegen. next steps / reproduction steps are unclear.
  • Let's take a look at this one as a group

// TODO: Support proper deserialization of http response query

  • unclear what is improper about our deserialization
  • let's take a look at this one as a group

// TODO: Make this pluggable so that this code can exist in aws-sdk-swift

  • unclear why we would want this code in aws-sdk-swift as it appears to be a general smithy function that is not specific to AWS

TODO("Organize test suites of smithy-swift and aws-sdk-swift and see if this class and consumers of this class should be moved to aws-sdk-swift OR AWS protocol tests in aws-sdk-swift should be moved to smithy-swift.")

  • written by @sichanyoo
  • Let's discuss as a group. Maybe we need or have a backlog ticket on examining the state of smithy-swift and what can and should be moved to aws-sdk-swift and vice versa.

Use Case

Clean up repository.

Proposed Solution

Remove unnecessary TODOs to clean up repo.

Other Information

Related issue: #710

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@dayaffe dayaffe added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 31, 2023
@sichanyoo
Copy link
Contributor

sichanyoo commented Nov 6, 2023

This document will be moved to Quip, and be edited / revised before being put public again. The document has to be updated with latest design.

@sichanyoo
Copy link
Contributor

// TODO create resource type - check for other SDKs whether this is common across SDKs

@jbelkins jbelkins removed the needs-triage This issue or PR still needs to be triaged. label Nov 21, 2023
@dayaffe
Copy link
Collaborator Author

dayaffe commented Nov 24, 2023

DESIGN.md document will be deleted. I have moved a copy of the raw file to /7wBGA7k8jGyM/Smithy-Swift-SDK-DESIGNmd

@jbelkins
Copy link
Contributor

jbelkins commented Dec 5, 2023

Released in AWS SDK for Swift 0.32.0

@jbelkins jbelkins closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants