-
Notifications
You must be signed in to change notification settings - Fork 881
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
Convert aws sdk 2.2 sqs tracing tests from groovy to java #11240
Conversation
...src/test/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/Aws2SqsTracingTest.java
Outdated
Show resolved
Hide resolved
...2/library/src/test/java/io/opentelemetry/instrumentation/awssdk/v2_2/Aws2SqsTracingTest.java
Outdated
Show resolved
Hide resolved
...2/library/src/test/java/io/opentelemetry/instrumentation/awssdk/v2_2/Aws2SqsTracingTest.java
Outdated
Show resolved
Hide resolved
...g/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/AbstractAws2SqsTracingTest.java
Show resolved
Hide resolved
return map; | ||
} | ||
|
||
ReceiveMessageRequest receiveMessageRequest = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It had better to define the those fields in a class together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify what you mean for this one? I moved the queueUrl
field closer to these methods that use it, but I'm not sure if that's what you are suggesting. Are you saying I should move these request fields into a new class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify what you mean for this one? I moved the
queueUrl
field closer to these methods that use it, but I'm not sure if that's what you are suggesting. Are you saying I should move these request fields into a new class?
What I actually mean is that all the fields in a class are defined in a piece of code, and then it is followed by some method definitions. I personally think this will make a class look clearer, for example, it will be easier to check whether some fields are used or whether the writing complies with the specification:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that makes sense, thank you for the explanation
Related to #7195