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

[ISSUE #3892] SendMessageRequestHeader, SendMessageBatchRequestHeader and SendMessageBatchV2RequestHeader have almost same body #3893

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pandaapo
Copy link
Member

Fixes #3892.

Motivation

See "Enhancement Request" in issue.

Modifications

See "Describe the solution you'd like" in issue.

Documentation

  • Does this pull request introduce a new feature? no

Copy link
Contributor

@Alonexc Alonexc left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #3893 (e4b2db9) into master (0ce8d57) will increase coverage by 0.31%.
The diff coverage is 78.16%.

❗ Current head e4b2db9 differs from pull request most recent head d3129d8. Consider uploading reports for the commit d3129d8 to get more accurate results

@@             Coverage Diff              @@
##             master    #3893      +/-   ##
============================================
+ Coverage     14.24%   14.55%   +0.31%     
- Complexity     1320     1350      +30     
============================================
  Files           579      580       +1     
  Lines         28967    28811     -156     
  Branches       2802     2799       -3     
============================================
+ Hits           4125     4194      +69     
+ Misses        24450    24225     -225     
  Partials        392      392              
Impacted Files Coverage Δ
.../header/message/SendMessageBatchRequestHeader.java 0.00% <0.00%> (ø)
...eader/message/SendMessageBatchV2RequestHeader.java 0.00% <0.00%> (ø)
.../http/header/message/SendMessageRequestHeader.java 0.00% <0.00%> (ø)
.../http/header/message/BaseSendMsgRequestHeader.java 83.95% <83.95%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.

If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.

@github-actions github-actions bot added the Stale label Apr 22, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 78.16092% with 19 lines in your changes missing coverage. Please review.

Project coverage is 14.55%. Comparing base (0ce8d57) to head (e4b2db9).
Report is 533 commits behind head on master.

Current head e4b2db9 differs from pull request most recent head d3129d8

Please upload reports for the commit d3129d8 to get more accurate results.

Files Patch % Lines
.../http/header/message/BaseSendMsgRequestHeader.java 83.95% 13 Missing ⚠️
.../header/message/SendMessageBatchRequestHeader.java 0.00% 2 Missing ⚠️
...eader/message/SendMessageBatchV2RequestHeader.java 0.00% 2 Missing ⚠️
.../http/header/message/SendMessageRequestHeader.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3893      +/-   ##
============================================
+ Coverage     14.24%   14.55%   +0.31%     
- Complexity     1320     1350      +30     
============================================
  Files           579      580       +1     
  Lines         28967    28811     -156     
  Branches       2802     2799       -3     
============================================
+ Hits           4125     4194      +69     
+ Misses        24450    24225     -225     
  Partials        392      392              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Pil0tXia Pil0tXia left a comment

Choose a reason for hiding this comment

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

Overall, good for me.

Comment on lines +35 to +39
*/
public abstract class BaseSendMsgRequestHeader extends Header {

//request code
private String code;
Copy link
Member

Choose a reason for hiding this comment

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

How about using the AbstractSendMsgRequestHeader naming, to match the naming of other classes?

Comment on lines +97 to +107
public void setCode(String code) {
this.code = code;
}

public String getLanguage() {
return language;
}

public void setLanguage(String language) {
this.language = language;
}
Copy link
Member

Choose a reason for hiding this comment

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

How about using the @Data annotation to save space on those getters and setters?

@Pil0tXia Pil0tXia changed the title [ISSUE #3892]SendMessageRequestHeader, SendMessageBatchRequestHeader and SendMessageBatchV2RequestHeader have almost same body [ISSUE #3892] SendMessageRequestHeader, SendMessageBatchRequestHeader and SendMessageBatchV2RequestHeader have almost same body Apr 23, 2024
@github-actions github-actions bot removed the Stale label Apr 23, 2024
Copy link
Contributor

It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.

If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.

@github-actions github-actions bot added Stale and removed Stale labels Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants