Skip to content

Conversation

@maximzgardan
Copy link
Contributor

@maximzgardan maximzgardan commented Aug 5, 2023

Fixes #4262.

Motivation

Modifications

located at:
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/grpc/common/EventMeshCloudEventUtils.java

1.Extract

try {
            return cloudEvent.getAttributesOrThrow(ProtocolKey.ENV).getCeString();
        } catch (Exception e) {
            return defaultValue;
        }

as a method private static String getValue(CloudEvent cloudEvent, String defaultValue, String protocolKey) .

2.Replace Optional.ofNullable() with 'Optional.of()' - Passing a non-null argument to 'Optional'.[366,383]

3.Remove Condition 'protoData == null' and 'binaryData == null' is always 'false'.[370,374]

4.Remove unnecessary return as the last statement in a 'void' method[416].

Documentation

  • Does this pull request introduce a new feature? no

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!

Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!

Want to get closer to the community?

WeChat Assistant WeChat Public Account Slack
Join Slack Chat

Mailing Lists:

Name Description Subscribe Unsubscribe Archive
Users User support and questions mailing list Subscribe Unsubscribe Mail Archives
Development Development related discussions Subscribe Unsubscribe Mail Archives
Commits All commits to repositories Subscribe Unsubscribe Mail Archives
Issues Issues or PRs comments and reviews Subscribe Unsubscribe Mail Archives

@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Merging #4337 (d397a02) into master (19dd2b1) will decrease coverage by 0.17%.
Report is 2 commits behind head on master.
The diff coverage is 90.32%.

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

@@             Coverage Diff              @@
##             master    #4337      +/-   ##
============================================
- Coverage     16.35%   16.18%   -0.17%     
- Complexity     1362     1363       +1     
============================================
  Files           594      594              
  Lines         25393    25343      -50     
  Branches       2393     2391       -2     
============================================
- Hits           4152     4103      -49     
+ Misses        20818    20815       -3     
- Partials        423      425       +2     
Files Changed Coverage Δ
...protocol/grpc/common/EventMeshCloudEventUtils.java 66.34% <90.32%> (-9.66%) ⬇️

... and 7 files with indirect coverage changes

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

mxsm
mxsm previously approved these changes Aug 6, 2023
Copy link
Member

@mxsm mxsm left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Welcome to EventMesh community.

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.

LGTM

mxsm
mxsm previously approved these changes Aug 7, 2023
@xwm1992 xwm1992 merged commit 68c8f23 into apache:master Aug 8, 2023
xuhongjia pushed a commit to Deckers-Ohana/eventmesh that referenced this pull request Mar 13, 2025
…che#4337)

* issue-4262 Enhancement Request EventMeshCloudEventUtils

* issue-4262 Enhancement Request EventMeshCloudEventUtils move on top method getValue

* issue-4262 Enhancement Request EventMeshCloudEventUtils remove redundant else for Optional.of

---------

Co-authored-by: maxim.zgardan <maxim.zgardan@accenture.com>
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.

[Enhancement] Some optimizations to EventMeshCloudEventUtils.

7 participants