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 #1193

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

chore: Clean up unnecessary TODOs #1193

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

Comments

@dayaffe
Copy link
Collaborator

dayaffe commented Oct 26, 2023

Describe the feature

The following TODOs can be removed as they are no longer necessary along with 1 small refactoring change

Use Case

We should address or remove all TODOs prior to GA.

Proposed Solution

Remove the following:

// TODO: remove once we have all the dependencies in the maven-publish plugin (build.gradle.kts, exclude(group = “brazil”))

  • Confirmed with @jbelkins that we do not have plans to move all of our dependencies to maven-publish plugin

// TODO: Subject to change when we figure out which strategy to use for XML // TODO: Subject to change if Foundation dependency is removed (XML Customizations)

// TODO ~ Implementation of embedded presigned URLs is TBD (PresignerGenerator.kt)

  • Discussed with @jbelkins. Consensus is to eliminate this as the description of work and reference to "embedded presigned URL" is unclear.

// TODO: We should support all types in our presignable operations

  • Discussed with @jbelkins: pre-signed URLs are customizations we support for a select few operations & it is unnecessary to support types that are not needed.

// TODO: add integration tests that automatically test that SSO crednetials provider correctly exchanges SSO token for temporary AWS credentails.

/// FIPS + path-only (TODO: consider making this an error) (EndpointResolverTest.swift)

  • This TODO was auto-generated. It is a suggestion to change an implementation based on the fact that we are not throwing an error in a test case and instead have actual logic. Deferring to the correctness of the implementation.

Address the following small changes:

// TODO: move to proper location once we figure out where that should be

  • Refactor re-used helper method for Middleware tests into a separate utility class/file

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 26, 2023
@dayaffe dayaffe self-assigned this Oct 30, 2023
@dayaffe dayaffe added this to the GA milestone Oct 30, 2023
@dayaffe dayaffe removed the needs-triage This issue or PR still needs to be triaged. label Oct 30, 2023
@dayaffe dayaffe closed this as completed Oct 31, 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

1 participant