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

[Trivial] Fix build references to deleted ServerHandlerImp #4592

Merged
merged 1 commit into from Jun 28, 2023

Conversation

ximinez
Copy link
Collaborator

@ximinez ximinez commented Jun 28, 2023

High Level Overview of Change

  • Fixes the build by changing references to ServerHandlerImp to the more correct ServerHandler.

Context of Change

Type of Change

  • [X ] Bug fix (non-breaking change which fixes an issue)

Before / After

Before: rippled doesn't build.
After: It does.

Future Tasks

Dedicating resources to wiping out all of the levelization issues across the board would help with this kind of thing.

* Commits 0b812cd (XRPLF#4427) and 11e914f (XRPLF#4516) conflict. The first added
  references to `ServerHandlerImp` in files outside of that class's
  organizational unit (which is technically incorrect). The second
  removed `ServerHandlerImp`, but was not up to date with develop. This
  results in the build failing in all circumstances.
* Fixes the build by changing references to `ServerHandlerImp` to
  the more correct `ServerHandler`.
@ximinez ximinez requested a review from scottschurr June 28, 2023 18:04
@ximinez ximinez changed the title Fix build references to deleted ServerHandlerImp [Trivial] Fix build references to deleted ServerHandlerImp Jun 28, 2023
@intelliot intelliot requested a review from ckeshava June 28, 2023 18:30
@@ -64,8 +64,8 @@
#include <ripple/resource/ResourceManager.h>
#include <ripple/rpc/BookChanges.h>
#include <ripple/rpc/DeliveredAmount.h>
#include <ripple/rpc/ServerHandler.h>
#include <ripple/rpc/impl/RPCHelpers.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we strictly follow the lexicographic order, shouldn't the import order be reversed?
alphabetically, S comes after i. While ordering the imports, are we keeping all the impl imports together?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It depends on whether the lexicographic order is case insensitive or not. Lower case "i" comes after capital "S".

Copy link
Collaborator

Choose a reason for hiding this comment

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

okay, didn't think of that :)

@intelliot intelliot merged commit 4111382 into XRPLF:develop Jun 28, 2023
15 checks passed
@ximinez ximinez deleted the fix-serverhandler branch June 28, 2023 20:36
Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

A bit late, but 👍 from me.

ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Jul 10, 2023
* Commits 0b812cd (XRPLF#4427) and 11e914f (XRPLF#4516) conflict. The first added
  references to `ServerHandlerImp` in files outside of that class's
  organizational unit (which is technically incorrect). The second
  removed `ServerHandlerImp`, but was not up to date with develop. This
  results in the build failing.
* Fixes the build by changing references to `ServerHandlerImp` to
  the more correct `ServerHandler`.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
* Commits 0b812cd (XRPLF#4427) and 11e914f (XRPLF#4516) conflict. The first added
  references to `ServerHandlerImp` in files outside of that class's
  organizational unit (which is technically incorrect). The second
  removed `ServerHandlerImp`, but was not up to date with develop. This
  results in the build failing.
* Fixes the build by changing references to `ServerHandlerImp` to
  the more correct `ServerHandler`.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
* Commits 0b812cd (XRPLF#4427) and 11e914f (XRPLF#4516) conflict. The first added
  references to `ServerHandlerImp` in files outside of that class's
  organizational unit (which is technically incorrect). The second
  removed `ServerHandlerImp`, but was not up to date with develop. This
  results in the build failing.
* Fixes the build by changing references to `ServerHandlerImp` to
  the more correct `ServerHandler`.
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.

None yet

5 participants