Skip to content

Conversation

@JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Feb 1, 2025

PR Description

Fixes a bug in the server factory.

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - CHANGELOG comment has been added
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr
  • - If new feature a unit test has been added

Summary by Sourcery

Bug Fixes:

  • Fix a bug in the server factory where the incorrect server type was being instantiated.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Feb 1, 2025

Reviewer's Guide by Sourcery

The ServerFactory class was updated to instantiate the ProxyBasicZMQ class instead of the Proxy class. This change was made to correct a bug in the server factory.

Class diagram showing the server factory and proxy relationship changes

classDiagram
    class IServer {
        <<interface>>
    }
    class ServerFactory {
        -m_log_context
        +create(socket_options, socket_credentials) IServer
    }
    class Proxy {
        +Proxy(socket_options, socket_credentials, log_context)
    }
    class ProxyBasicZMQ {
        +ProxyBasicZMQ(socket_options, socket_credentials, log_context)
    }
    ServerFactory ..> IServer: creates
    Proxy ..|> IServer: implements
    ProxyBasicZMQ ..|> IServer: implements
    note for ServerFactory "Changed to create ProxyBasicZMQ instead of Proxy"
Loading

File-Level Changes

Change Details Files
The ServerFactory now instantiates the ProxyBasicZMQ class.
  • The Proxy.hpp include was removed.
  • The ProxyBasicZMQ.hpp include was added.
  • The ServerFactory::create method now instantiates a ProxyBasicZMQ object instead of a Proxy object.
common/source/ServerFactory.cpp

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@JoshuaSBrown JoshuaSBrown self-assigned this Feb 1, 2025
@JoshuaSBrown JoshuaSBrown added the Type: Bug Something isn't working label Feb 1, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @JoshuaSBrown - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Please provide a more detailed description of the bug and why using ProxyBasicZMQ instead of Proxy fixes it. This context is important for future maintenance.
  • Add unit tests to verify the factory correctly instantiates ProxyBasicZMQ and that it behaves as expected.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@JoshuaSBrown
Copy link
Collaborator Author

I think this is not passing because of corruption in a build layer of one of the docker containers....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Common, Proxy] - Bug proxy basic is not being returned from server factory

3 participants