Skip to content

Cleanup pending task comments #569

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

Merged
merged 4 commits into from
Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package software.amazon.awssdk.auth.signer;

import java.time.Instant;
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.auth.credentials.AwsCredentials;
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
Expand All @@ -29,8 +28,6 @@
* AWS-specific signing attributes attached to the execution. This information is available to {@link ExecutionInterceptor}s and
* {@link Signer}s.
*/
@ReviewBeforeRelease("We should also consider making some of the SDK/AWS-owned set of attributes part of the immutable context"
+ "if we don't want the interceptors to modify them.")
@SdkProtectedApi
public final class AwsSignerExecutionAttribute extends SdkExecutionAttribute {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ protected final ShapeModel generateShapeModel(String javaClassName, String shape
final List<String> enumValues = shape.getEnumValues();
if (enumValues != null && !enumValues.isEmpty()) {
for (String enumValue : enumValues) {
// TODO handle useRealName if explicitly mentioned in
Copy link
Contributor

Choose a reason for hiding this comment

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

did we already finish this todo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This TODO is from v1 which was added 3 years back. I think we never prioritized it. So removed it as its not specific to v2 and not a blocker for GA.

// the customization.
shapeModel.addEnum(
new EnumModel(getNamingStrategy().getEnumValueName(enumValue), enumValue));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public final class DocumentationUtils {
private static final String DEFAULT_FLUENT_RETURN =
"Returns a reference to this object so that method calls can be chained together.";

//TODO kylthoms@: probably should move this to a custom config in each service
//TODO probably should move this to a custom config in each service
private static final Set<String> SERVICES_EXCLUDED_FROM_CROSS_LINKING = new HashSet<>(Arrays.asList(
"apigateway", "budgets", "cloudsearch", "cloudsearchdomain",
"discovery", "elastictranscoder", "es", "glacier", "importexport",
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ public class ShapeModel extends DocumentationModel implements HasDeprecation {
private String requestSignerClassFqcn;

private List<MemberModel> members;
// Any constructor in addition to the default no-arg
private List<ConstructorModel> additionalConstructors;
private List<EnumModel> enums;

private VariableModel variable;
Expand Down Expand Up @@ -289,22 +287,6 @@ public void addMember(MemberModel member) {
members.add(member);
}

@JsonIgnore
public List<ConstructorModel> getAdditionalConstructors() {
return additionalConstructors;
}

public void setAdditionalConstructors(List<ConstructorModel> additionalConstructors) {
this.additionalConstructors = additionalConstructors;
}

public void addConstructor(ConstructorModel constructor) {
if (this.additionalConstructors == null) {
this.additionalConstructors = new ArrayList<ConstructorModel>();
}
this.additionalConstructors.add(constructor);
}

public List<EnumModel> getEnums() {
return enums;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package software.amazon.awssdk.codegen.poet;

import com.squareup.javapoet.ClassName;
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;

/**
Expand Down Expand Up @@ -69,7 +68,6 @@ public ClassName getClientClass(String className) {
* Example: If operationName is "ListTables", then the response type of the paginated operation
* will be "ListTablesIterable" class.
*/
@ReviewBeforeRelease("Naming of response shape for paginated APIs")
public ClassName getResponseClassForPaginatedSyncOperation(String operationName) {
return ClassName.get(model.getMetadata().getFullPaginatorsPackageName(), operationName + "Iterable");
}
Expand All @@ -81,7 +79,6 @@ public ClassName getResponseClassForPaginatedSyncOperation(String operationName)
* Example: If operationName is "ListTables", then the async response type of the paginated operation
* will be "ListTablesPublisher" class.
*/
@ReviewBeforeRelease("Naming of response shape for paginated APIs")
public ClassName getResponseClassForPaginatedAsyncOperation(String operationName) {
return ClassName.get(model.getMetadata().getFullPaginatorsPackageName(), operationName + "Publisher");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public Optional<ClassName> copierClassFor(MemberModel memberModel) {
return Optional.of(ClassName.get(StandardMemberCopier.class));
}

// FIXME: Ugly hack, but some services (Health) have shapes with names
// FIXME: Some services (Health) have shapes with names
// that differ only in the casing of the first letter, and generating
// classes for them breaks on case insensitive filesystems...
String shapeName = memberModel.getC2jShape();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

package software.amazon.awssdk.codegen.utils;

import software.amazon.awssdk.annotations.ReviewBeforeRelease;

public final class PaginatorUtils {

private PaginatorUtils() {
Expand All @@ -26,7 +24,6 @@ private PaginatorUtils() {
* @param methodName Name of a method in sync client
* @return the name of the auto-pagination enabled operation
*/
@ReviewBeforeRelease("Naming of paginated APIs")
public static String getPaginatedMethodName(String methodName) {
return methodName + "Paginator";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private boolean shouldPutParamsInBody(SdkHttpFullRequest.Builder input,
!CollectionUtils.isNullOrEmpty(input.rawQueryParameters());
}

// TODO FIXME hacky hack
// TODO FIXME
@ReviewBeforeRelease("SigV2 expects query params to be signed as query params despite being in the body. Moving" +
" before signing breaks SimpleDB which still uses SigV2. Probably the best thing to do is" +
" make the SigV2 signer be aware that params are being moved into the body and move them back out" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package software.amazon.awssdk.core.internal.http.pipeline.stages;

import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.http.ExecutionContext;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
Expand Down Expand Up @@ -76,8 +75,6 @@ private void updateInterceptorContext(SdkHttpFullRequest request, ExecutionConte
*
* @return True if request should be signed, false if not.
*/
@ReviewBeforeRelease("add back credential check (credentials != null || signer instanceof CanHandleNullCredentials) when "
+ "refactoring signer")
private boolean shouldSign(Signer signer) {
return signer != null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
@SdkPublicApi
public interface SdkPublisher<T> extends Publisher<T> {

// TODO Should we return the last response instead of Void?
default CompletableFuture<Void> forEach(Consumer<T> consumer) {
CompletableFuture<Void> future = new CompletableFuture<>();
subscribe(new SequentialSubscriber<>(consumer, future));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
package software.amazon.awssdk.core.util;

import java.util.stream.Stream;
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.annotations.SdkProtectedApi;

@ReviewBeforeRelease("We should consider migrating this and other related util classes to the utils package.")
@SdkProtectedApi
public final class ComparableUtils {
private ComparableUtils() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import org.junit.Before;
import org.junit.Test;
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.dynamodb.model.ListTablesRequest;
import software.amazon.awssdk.services.dynamodb.model.ListTablesResponse;
Expand All @@ -38,15 +37,13 @@ public void setup() {
}

@Test
@ReviewBeforeRelease("Response metadata has been broken by client/interface refactoring. Fix before release")
public void responseMetadataInBaseResultIsSameAsMetadataCache() {
ListTablesRequest request = ListTablesRequest.builder().build();
ListTablesResponse result = dynamoDB.listTables(request);
assertThat(result.sdkHttpResponse().headers()).isNotNull();
}

@Test
@ReviewBeforeRelease("Response metadata has been broken by client/interface refactoring. Fix before release")
public void httpMetadataInBaseResultIsValid() {
ListTablesResponse result = dynamoDB.listTables(ListTablesRequest.builder().build());
assertThat(result.sdkHttpResponse().statusCode()).isEqualTo(200);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.junit.Assert.assertTrue;

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
Expand All @@ -36,7 +35,6 @@ public void setup() {
}

@Test
@Ignore // FIXME: Fails with "region cannot be null"
public void clientCreatedWithConstructor_RegistersGlobalHandlers() {
assertFalse(TestGlobalExecutionInterceptor.wasCalled());
DynamoDbClient client = DynamoDbClient.builder()
Expand All @@ -48,7 +46,6 @@ public void clientCreatedWithConstructor_RegistersGlobalHandlers() {
}

@Test
@Ignore // FIXME: Fails with "region cannot be null"
public void clientCreatedWithBuilder_RegistersGlobalHandlers() {
assertFalse(TestGlobalExecutionInterceptor.wasCalled());
DynamoDbClient client = DynamoDbClient.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Map;
import java.util.Map.Entry;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
Expand Down Expand Up @@ -295,7 +294,6 @@ public void testExpected_EmptyAttributeName() {
}

@Test
@Ignore // FIXME: failes with "region cannot be null"
public void testExpected_DuplicateAttribute() {
Table fakeTable = new Table(DynamoDbClient.builder().region(Region.US_WEST_2).build(), "fake-table");
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Map;
import java.util.Map.Entry;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
Expand Down Expand Up @@ -268,7 +267,6 @@ public void testScanFilter_EmptyAttributeName() {
}

@Test
@Ignore // FIXME: fails with "region cannot be null"
public void testScanFilter_DuplicateAttribute() {
Table fakeTable = new Table(DynamoDbClient.builder().region(Region.US_WEST_2).build(), "fake-table");
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.Collection;
import java.util.List;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
import software.amazon.awssdk.regions.Region;
Expand All @@ -42,7 +41,6 @@
/**
* Tests on the DynamoDBMapper.generateCreateTableRequest method.
*/
@Ignore // FIXME: setup fails with "region cannot be null"
public class GenerateCreateTableRequestTest extends DynamoDBTestBase {

private static DynamoDbMapper mapper;
Expand Down
4 changes: 0 additions & 4 deletions test/test-utils/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down

This file was deleted.