From 79f0ecc2f4c59a552d4f844ad0ccfa3f952555f1 Mon Sep 17 00:00:00 2001 From: Peter Song Date: Mon, 13 Oct 2025 16:53:02 -0700 Subject: [PATCH 1/4] Add s3-specific instructions for copilot --- .../s3-generation-instructions.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/instructions/s3-generation-instructions.md diff --git a/.github/instructions/s3-generation-instructions.md b/.github/instructions/s3-generation-instructions.md new file mode 100644 index 000000000000..9130c5603257 --- /dev/null +++ b/.github/instructions/s3-generation-instructions.md @@ -0,0 +1,32 @@ +--- +applyto: "sdk/src/Services/S3/**" +--- + + +# Purpose + +This document provides a comprehensive guideline for how to review S3 generation pull requests. S3 generation pull requests are pull requests which convert custom s3 code to generated s3 code. This means that the code is no longer written by hand and is generated by the generator located in `generator/ServiceClientGeneratorLib/**`. A generated file exists under the generated folder while a custom file exists under the custom folder. Here is an example of a file that has been moved from custom to generated: + +* `sdk/src/Services/S3/Custom/Model/InitiateMultipartUploadRequest.cs` is the custom file +* `sdk/src/Services/S3/Generated/Model/InitiateMultipartUploadRequest.cs` is the generated file the code is moved to. + +It isn't always possible to fully generate the s3 code so in some cases the logic will be split between a custom marshaller and a generated marshaller: + +Here is an example of s3 marshaller logic split between two files +* `sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/GetObjectMetadataRequestMarshaller.cs` +* `sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/GetObjectMetadataRequestMarshaller.cs` + +When a custom file is split between two files (custom and generated) the custom and generated should be looked as a whole when analyzing the logic. + +The same can be said about unmarshallers. Here is an example of S3 unmarshaller logic that is split between two files: +* `sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/ListObjectsV2ResponseUnmarshaller.cs` +* `sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/ListObjectsV2ResponseUnmarshaller.cs` + +Similarly to the s3 marshallers, when the custom file is split between two files (custom and generator) the custom and generated should be looked as a whole when analyzing the logic. + +# Guidelines for reviewing S3 Generation Pull Requests +Your job is to make sure that there are no breaking changes between the custom code and the generated code (whether it be a combination of custom + generated). Some things that would cause breaking changes are: +1. logical conditions that existed in the older code have been changed or removed. +2. getter and setter code have changed, disregarding private variable name changes such as _type and type. +3. properties that were set to values that are no longer set. +4. changes in the response output. From 74ee6fc04bfbd6bda262979cd2256bd89c5ee135 Mon Sep 17 00:00:00 2001 From: Peter Song Date: Mon, 13 Oct 2025 17:05:04 -0700 Subject: [PATCH 2/4] Add more explanations on split marshall/unmarshall files --- .github/instructions/s3-generation-instructions.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/instructions/s3-generation-instructions.md b/.github/instructions/s3-generation-instructions.md index 9130c5603257..cb6a9fb9e205 100644 --- a/.github/instructions/s3-generation-instructions.md +++ b/.github/instructions/s3-generation-instructions.md @@ -12,16 +12,21 @@ This document provides a comprehensive guideline for how to review S3 generation It isn't always possible to fully generate the s3 code so in some cases the logic will be split between a custom marshaller and a generated marshaller: -Here is an example of s3 marshaller logic split between two files +Here is an example of s3 marshaller logic split between two files: + * `sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/GetObjectMetadataRequestMarshaller.cs` * `sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/GetObjectMetadataRequestMarshaller.cs` +In the example above, the logic that can't be generated has been moved to a custom partial method inside of the custom file called `PostMarshallCustomization`, which the generated marshaller then calls. + When a custom file is split between two files (custom and generated) the custom and generated should be looked as a whole when analyzing the logic. The same can be said about unmarshallers. Here is an example of S3 unmarshaller logic that is split between two files: * `sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/ListObjectsV2ResponseUnmarshaller.cs` * `sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/ListObjectsV2ResponseUnmarshaller.cs` +In the example above, the logic that can't be generated has been moved to a custom method called `CustomContentsUnmarshall` that is called from within the generated unmarshaller. The name of the custom method will differ depending on what the custom method is doing, but the pattern of the generated unmarshaller calling a custom method will be commonly found within S3 generation pull requests. + Similarly to the s3 marshallers, when the custom file is split between two files (custom and generator) the custom and generated should be looked as a whole when analyzing the logic. # Guidelines for reviewing S3 Generation Pull Requests From 20b36dfbfb65b4f74832f112db54973eabacd7a3 Mon Sep 17 00:00:00 2001 From: Peter Song Date: Mon, 13 Oct 2025 17:34:12 -0700 Subject: [PATCH 3/4] rename file to s3.instructions.md --- .../{s3-generation-instructions.md => s3.instructions.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/instructions/{s3-generation-instructions.md => s3.instructions.md} (100%) diff --git a/.github/instructions/s3-generation-instructions.md b/.github/instructions/s3.instructions.md similarity index 100% rename from .github/instructions/s3-generation-instructions.md rename to .github/instructions/s3.instructions.md From 9fa4c82e8f193ff859d33c1ac0c48a7fc6b9c875 Mon Sep 17 00:00:00 2001 From: Peter Song Date: Mon, 13 Oct 2025 17:37:37 -0700 Subject: [PATCH 4/4] copilot suggestions --- .github/instructions/s3.instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/instructions/s3.instructions.md b/.github/instructions/s3.instructions.md index cb6a9fb9e205..11eb9ffd4309 100644 --- a/.github/instructions/s3.instructions.md +++ b/.github/instructions/s3.instructions.md @@ -27,10 +27,10 @@ The same can be said about unmarshallers. Here is an example of S3 unmarshaller In the example above, the logic that can't be generated has been moved to a custom method called `CustomContentsUnmarshall` that is called from within the generated unmarshaller. The name of the custom method will differ depending on what the custom method is doing, but the pattern of the generated unmarshaller calling a custom method will be commonly found within S3 generation pull requests. -Similarly to the s3 marshallers, when the custom file is split between two files (custom and generator) the custom and generated should be looked as a whole when analyzing the logic. +Similarly to the s3 marshallers, when the custom file is split between two files (custom and generated) the custom and generated should be looked as a whole when analyzing the logic. # Guidelines for reviewing S3 Generation Pull Requests -Your job is to make sure that there are no breaking changes between the custom code and the generated code (whether it be a combination of custom + generated). Some things that would cause breaking changes are: +Your job is to ensure there are no breaking changes when comparing the custom code to the generated code (or a combination of custom + generated code). Some things that would cause breaking changes are: 1. logical conditions that existed in the older code have been changed or removed. 2. getter and setter code have changed, disregarding private variable name changes such as _type and type. 3. properties that were set to values that are no longer set.