Skip to content

[ISSUE #4195] Do some code optimization.[SubscribeProcessor]#4383

Merged
xwm1992 merged 6 commits intoapache:masterfrom
gautamsagar99:gautamsagar99/issue#4195
Aug 25, 2023
Merged

[ISSUE #4195] Do some code optimization.[SubscribeProcessor]#4383
xwm1992 merged 6 commits intoapache:masterfrom
gautamsagar99:gautamsagar99/issue#4195

Conversation

@gautamsagar99
Copy link
Contributor

Fixes #4195

Motivation

To solve below two issues:

  1. transient is used to mark fields in a Serializable class which will not be written out to file (or stream). In a class that does not implement Serializable, this modifier is simply wasted keystrokes, and should be removed.
  2. Dereference of 'subscriptionItems' may produce 'NullPointerException'

Modifications

  1. I have added a static keyword.
  2. I have added Objects.requireNonNull() for the subscriptionItems list.

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)
  • If a feature is not applicable for documentation, explain why? (Fixed small issue)

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

@pandaapo pandaapo changed the title ISSUE #4195: Update SubscribeProcessor.java [ISSUE #4195] Do some code optimization.[SubscribeProcessor] Aug 18, 2023
@pandaapo
Copy link
Member

The rest of CI errors are being fixed by PR #4063.
You can wait for that PR to be merged, and then you pull newest master code and merge it into this branch.

@gautamsagar99
Copy link
Contributor Author

I would like to extend my sincere thanks to @pandaapo and @Alonexc for their valuable assistance in resolving this PR. Their insights and guidance were instrumental in tackling the challenges effectively. 🙌🏼 This being my first open-source project, their help was incredibly valuable and made the experience much smoother.

@Alonexc
Copy link
Contributor

Alonexc commented Aug 18, 2023

Welcome to the EventMesh community. ci error is not related to your pr, but you can try to fix it.

@pandaapo
Copy link
Member

@gautamsagar99 PR #4063 was merged. You can update your master branch and merge it into this. Then CI errors will be resolved.

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.

@gautamsagar99 Merge or rebase master branch to fix CI.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #4383 (48293df) into master (8f584d5) will increase coverage by 0.02%.
Report is 7 commits behind head on master.
The diff coverage is 46.15%.

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

@@             Coverage Diff              @@
##             master    #4383      +/-   ##
============================================
+ Coverage     17.81%   17.84%   +0.02%     
- Complexity     1512     1514       +2     
============================================
  Files           601      602       +1     
  Lines         25495    25520      +25     
  Branches       2400     2400              
============================================
+ Hits           4543     4555      +12     
- Misses        20514    20526      +12     
- Partials        438      439       +1     
Files Changed Coverage Δ
...nnector/s3/source/connector/S3SourceConnector.java 0.00% <0.00%> (ø)
...re/protocol/grpc/processor/SubscribeProcessor.java 0.00% <0.00%> (ø)
.../tcp/client/session/retry/EventMeshTcpRetryer.java 0.00% <0.00%> (ø)
...pache/eventmesh/runtime/util/ThreadPoolHelper.java 75.00% <75.00%> (ø)

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

private final EventMeshGrpcServer eventMeshGrpcServer;

private final transient GrpcType grpcType = GrpcType.WEBHOOK;
private static final GrpcType grpcType = GrpcType.WEBHOOK;
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 changing “grpcType” to “GRPC_TYPE”?

@xwm1992 xwm1992 merged commit 51a1171 into apache:master Aug 25, 2023
xuhongjia pushed a commit to Deckers-Ohana/eventmesh that referenced this pull request Mar 13, 2025
…pache#4383)

* apache#4195 issue: Update SubscribeProcessor.java

* apache#4195 issue: Modified  SubscribeProcessor.java

* apache#4195 issue:Fixed SubscribeProcessor.java

* apache#4195 issue: Changed SubscribeProcessor.java

* apache#4195 issue:Resolved Syntax Error in SubscribeProcessor.java

* Commiting after removing CI Merge Errors
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] Do some code optimization.[SubscribeProcessor]

6 participants