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

[PROPOSAL] Separate generated code or remove "THIS CODE IS GENERATED" comments #218

Closed
AarjavP opened this issue Sep 13, 2022 · 3 comments
Closed

Comments

@AarjavP
Copy link

AarjavP commented Sep 13, 2022

What are you proposing?

Remove

//----------------------------------------------------
// THIS CODE IS GENERATED. MANUAL EDITS WILL BE LOST.
//----------------------------------------------------

where it does not make sense, or put the generated source code into a separate directory (source set) for clear separation.

How did you come up with this proposal?

I was browsing the code to see if I could reuse the models/classes for a kotlin client when I came across many of these comments. From #197 #182 opensearch-project/opensearch-clients#19 I see that the comment may be incorrect, at least for now.

What is the user experience going to be?

should be transparent to users of the library, but hopefully this makes the code more maintainable for contributors.

Why should it be built? Any reason not to?

Maybe keeping common package code together? But I think the benefits of having a separate directory outweigh this point, even if only to make sure the code generator does not overwrite non-generated files.

What will it take to execute?

time and review

What are remaining open questions?

I have not yet worked with smithy, so are there any considerations to be made for that given it may be used in the near future?
Also, I am assuming that the entirety of the client is not (or will not be) generated. Is that a correct assumption?

@AarjavP
Copy link
Author

AarjavP commented Sep 18, 2022

Should I go ahead and create PR for this?

@dblock
Copy link
Member

dblock commented Sep 19, 2022

Should I go ahead and create PR for this?

Please! No permission needed.

AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Oct 8, 2022
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Oct 8, 2022
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
AarjavP added a commit to AarjavP/opensearch-java that referenced this issue Nov 14, 2022
broken down into multiple commits as per opensearch-project#235

Signed-off-by: AarjavP <[email protected]>
@VachaShah
Copy link
Collaborator

Done in #598

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 a pull request may close this issue.

3 participants