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.

Sign up for GitHub

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

Jump to bottom

[MNEDC] Refactoring in terms of Architecture #178

Open
1 of 2 tasks
tiokim opened this issue Nov 16, 2020 · 8 comments · Fixed by #180
Open
1 of 2 tasks

[MNEDC] Refactoring in terms of Architecture #178

tiokim opened this issue Nov 16, 2020 · 8 comments · Fixed by #180
Assignees
@tiokim @MoonkiHong @suresh-lc
Labels
question Further information is requested refactoring Any tasks and issues w.r.t. the code refactoring

Comments

@tiokim
Copy link
Contributor

tiokim commented Nov 16, 2020 •
edited
Loading

I think that the mnedcmgr function should be under the discoverymgr since they are similar with respect to functionality.
How about moving the mnedcmgr code under the discoverymgr?
Below should be changed.

  • Change package name from mnedcmgr to mnedc or something.
  • The discoverymgr should call StartMNEDCServer() and StartMNEDCClient().

Any comments are welcome!
@MoonkiHong MoonkiHong added question Further information is requested refactoring Any tasks and issues w.r.t. the code refactoring labels Nov 16, 2020
@MoonkiHong
Copy link
Contributor

@ayush-kr @suresh-lc What about your opinion (from the contributors' perspective)?

@MoonkiHong
Copy link
Contributor

I think that the mnedcmgr function should be under the discoverymgr since they are similar with respect to functionality.
How about moving the mnedcmgr code under the discoverymgr?
Below should be changed.

  • Change package name from mnedcmgr to mnedc or something.
  • The discoverymgr should call StartMNEDCServer() and StartMNEDCClient().

Any comments are welcome!

@t25kim Thank you for a great suggestion. We will estimate its feasibility to migrate the functions of the mnedcmgr into discoverymgr with regarding contributors of mnedc.

@suresh-lc
Copy link
Contributor

@t25kim @MoonkiHong : We checked this modification. Seem to involve cyclical importing which Go doesn't support. So we are checking on changing the same and testing.

@MoonkiHong
Copy link
Contributor

@t25kim @MoonkiHong : We checked this modification. Seem to involve cyclical importing which Go doesn't support. So we are checking on changing the same and testing.

@suresh-lc Thank you for your prompt analysis. As you suggested, let us discuss further about the best approach once you completed the check-up on changing the same and testing stuff, especially about the cyclical importing which GoLang does not support. We will wait for your updates. ^^

ayush-kr added a commit to ayush-kr/edge-home-orchestration-go that referenced this issue Nov 19, 2020
@ayush-kr
Moved mnedc logic to discoverymgr, which fixes lf-edge#178
b5282e9 
Signed-off-by: ayush.kumar <[email protected]>
@ayush-kr ayush-kr mentioned this issue Nov 19, 2020
Merged
8 tasks
MoonkiHong pushed a commit that referenced this issue Nov 22, 2020
@ayush-kr
Moved mnedc logic to discoverymgr, which fixes #178 (#180)
1d45f03 
Signed-off-by: ayush.kumar <[email protected]>
- Since MNEDC responsibility is similar to the Discovery Manager, the mnedcmgr package has now been moved inside discoverymgr package and renamed to mnedc.
@MoonkiHong
Copy link
Contributor

This issue is still valid since the latest merged PR #178 resolved Change package name from mnedcmgr to mnedc or something..

@MoonkiHong MoonkiHong reopened this Nov 22, 2020
@suresh-lc
Copy link
Contributor

All the implementations have been done

@tiokim
Copy link
Contributor Author

tiokim commented Nov 15, 2021

Sorry. But this has not been resolved yet.

@tiokim tiokim reopened this Nov 15, 2021
@suresh-lc
Copy link
Contributor

Sorry. But this has not been resolved yet.

Ok i get you, lets check this how to go about.

@tiokim tiokim mentioned this issue Dec 14, 2021
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tiokim tiokim

@MoonkiHong MoonkiHong

@suresh-lc suresh-lc

Labels
question Further information is requested refactoring Any tasks and issues w.r.t. the code refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Moved mnedc logic to discoverymgr, which fixes #178 ayush-kr/edge-home-orchestration-go
3 participants
@tiokim @MoonkiHong @suresh-lc