Skip to content

Web server refactor#667

Merged
cindyyan317 merged 1 commit intoXRPLF:developfrom
cindyyan317:server
Jun 8, 2023
Merged

Web server refactor#667
cindyyan317 merged 1 commit intoXRPLF:developfrom
cindyyan317:server

Conversation

@cindyyan317
Copy link
Contributor

1 Separate web server with RPC. Web server becomes more generic.
2 Expand the WsBase to ConnectionBase, both RPC Executor and SubscriptionManager will send the response via ConnectionBase.
3 Add unittests
4 Add some logs

@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Merging #667 (91764cf) into develop (5d2c079) will increase coverage by 0.86%.
The diff coverage is n/a.

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

@@             Coverage Diff             @@
##           develop     #667      +/-   ##
===========================================
+ Coverage    30.23%   31.09%   +0.86%     
===========================================
  Files          140      149       +9     
  Lines         7436     7892     +456     
  Branches      4422     4683     +261     
===========================================
+ Hits          2248     2454     +206     
- Misses        3109     3145      +36     
- Partials      2079     2293     +214     

see 23 files with indirect coverage changes

@cindyyan317 cindyyan317 requested a review from godexsoft June 5, 2023 11:50
@cindyyan317 cindyyan317 marked this pull request as ready for review June 5, 2023 11:50
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

Leaving a few comments 👍
Overall looks good but needs a few tweaks here and there:

  1. remove most one line comments that are straight up useless and/or come from examples of asio
  2. Use doxygen comments to document what member functions do as well as on top of classes to briefly describe what they are for.

@cindyyan317 cindyyan317 requested a review from godexsoft June 6, 2023 13:30
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

Adding a few more comments and questions 👍 I think we are very close to merging this 🥳

@cindyyan317 cindyyan317 requested a review from godexsoft June 8, 2023 08:27
godexsoft
godexsoft previously approved these changes Jun 8, 2023
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

Looks good 👍🙏🚀

Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@cindyyan317 cindyyan317 merged commit 435db33 into XRPLF:develop Jun 8, 2023
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