-
Notifications
You must be signed in to change notification settings - Fork 25
Refactor the output component #2086
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
Draft
Molter73
wants to merge
2
commits into
mauro/ROX-28981-rework-collector-iservice-protobuf
Choose a base branch
from
mauro/refactor-output
base: mauro/ROX-28981-rework-collector-iservice-protobuf
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Refactor the output component #2086
Molter73
wants to merge
2
commits into
mauro/ROX-28981-rework-collector-iservice-protobuf
from
mauro/refactor-output
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f677d59
to
85cb9c6
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## mauro/ROX-28981-rework-collector-iservice-protobuf #2086 +/- ##
===================================================================================
Coverage 27.02% 27.02%
===================================================================================
Files 98 100 +2
Lines 6046 6046
Branches 2701 2701
===================================================================================
Hits 1634 1634
Misses 3693 3693
Partials 719 719
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
d733fff
to
908e931
Compare
85cb9c6
to
ffbe42c
Compare
908e931
to
8341cda
Compare
ffbe42c
to
efc1c53
Compare
8341cda
to
14555ea
Compare
efc1c53
to
6f05199
Compare
14555ea
to
0527c42
Compare
Simplify naming for classes in the output namespace. Create dedicated namespaces for gprc and log implementations of IClient.
6f05199
to
51fdfaa
Compare
0527c42
to
7d26ea8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a follow up to #2063.
This PR is focused on re-organizing the newly created CollectorOutput component into its own namespace. It is also grouped together with the sensor formatter, since they are tightly couple at the moment, but this might change in the near future.
A formal
IClient
interface is introduced and implementers of if are grouped into their own namespaces, this should make it easier to implement new output types by simply adding a new client in a separate namespace and implementing the interface. Right now theIClient
interface only accounts for processes, but it will be extended in the near future to take aconst std::variant&
in order for it to also handle network information. This should also make it more flexible for adding newer message types in the future.Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why you did not do so. (Valid reasons include "CI is sufficient" or "No testable changes")
In addition to reviewing your code, reviewers must also review your testing instructions, and make sure they are sufficient.
For more details, ref the Confluence page about this section.