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

Add an RBS signature for Aws::Query::Param #3150

Conversation

kazuyainoue0124
Copy link

Here is my first PR to the aws-sdk-core repository.
I kindly ask for your understanding if there are any mistakes.

I have added an RBS type definition file, starting with just one file as a trial.

If this PR is successfully merged, I would like to continue contributing by adding more RBS type definitions in the future.

Thank you for taking the time to review this. I look forward to your feedback.

@mullermp
Copy link
Contributor

Thanks for opening a pull request. We are happy to take RBS contributions. I'm wondering, why did you choose Aws::Query::Param specifically? I believe this a private API not intended to be used by customers. I think there is more value to RBS for public classes. That said, I'm not against taking this, but I'd like to see more than just this class added before we merge and bump the SDK version.

@kazuyainoue0124
Copy link
Author

Thank you for your feedback, and I apologize for my misunderstanding.

I initially chose Aws::Query::Param because I assumed that any class without # @api private was part of the public API. I now realize this was incorrect. Moving forward, I’ll focus on contributing RBS for public classes.

Are there any specific indicators within the project, such as directory structures or naming conventions, to distinguish public APIs from internal ones? Or is comparing with the official documentation the primary way to make this determination?

Thank you for your patience and guidance.

@mullermp
Copy link
Contributor

No worries :) You are correct that the API private documentation indicates that. Sometimes we've forgotten to add that comment, or it ends up not being necessary because it's already omitted from docs.

As a trial run, I think a good place to start might be to add RBS for the sigv4 gem since it is not too large and it's self contained. You could add RBS for the public errors, the request, the signature, and the signer and signer methods.

@kazuyainoue0124
Copy link
Author

Thank you for the helpful clarification and guidance!

I’ll take your suggestion and work on adding RBS for the sigv4 gem. It seems like a great starting point, and I’m excited to dive in!

To keep things clean, I’ll close this PR and submit a new one focused on the sigv4 gem. I’ll do my best to contribute, even though I still have a lot to learn!

Thanks again for your support!

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 this pull request may close these issues.

2 participants