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

Refactor control interface #345

Merged
merged 12 commits into from
Aug 15, 2024

Conversation

GabyUnalaq
Copy link
Contributor

@GabyUnalaq GabyUnalaq commented Aug 7, 2024

Refactoring decisions are explained in a comment inside #322.

Changes

Renamed some structs from the control_interface:

Before After
PipesChannelContext ControlInterface
PipesChannelContextError ControlInterfaceError
MockPipesChannelContext MockControlInterface
PipesChannelContextInfo ControlInterfaceInfo
MockPipesChannelContextInfo MockControlInterfaceInfo
PipesChannelTask ControlInterfaceTask
MockPipesChannelTask MockControlInterfaceTask

Moved files related to the control_interface:

Before After
src\control_interface\pipes_channel_context.rs src\control_interface.rs
src\control_interface\pipes_channel_context_info.rs src\control_interface\control_interface_info.rs
src\control_interface\pipes_channel_task.rs src\control_interface\control_interface_task.rs

Got rid of wildcard imports.

Definition of Done

The PR shall be merged only if all items mentioned in CONTRIBUTING.md have been followed. In case an item is not applicable as described, please provide a short explanation in the description.

Copy link
Contributor

@inf17101 inf17101 left a comment

Choose a reason for hiding this comment

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

The renamings are fine. Just one small variable name finding.

In addition, we shall refactor the duplicated methods "create_control_interface(...)" as well. I think it is not good, that the ControlInterfaceInfo struct contains a method "create_control_interface" in general.

agent/src/runtime_manager.rs Show resolved Hide resolved
@inf17101
Copy link
Contributor

Please test the control interface examples one times.
Check the code coverage if it has decreased.

@inf17101
Copy link
Contributor

inf17101 commented Aug 14, 2024

Please test the control interface examples one times. Check the code coverage if it has decreased.

I have tested the examples and their are working.
I have checked the test coverage and it is fine.

Copy link
Contributor

@windsource windsource left a comment

Choose a reason for hiding this comment

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

As far as I can see, it looks fine to me.

@inf17101
Copy link
Contributor

As far as I can see, it looks fine to me.

Ok thank you. I will step once through the swdd requirements. So please wait with the merge.

@inf17101
Copy link
Contributor

As far as I can see, it looks fine to me.

Ok thank you. I will step once through the swdd requirements. So please wait with the merge.

@windsource: I have adapted the swdds and fixed also some linkage of existing tests.

@inf17101 inf17101 mentioned this pull request Aug 14, 2024
1 task
@inf17101 inf17101 merged commit d56ab0e into eclipse-ankaios:main Aug 15, 2024
10 checks passed
@inf17101 inf17101 deleted the refactor_control_interface branch August 15, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants