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

Is endpointElement really required #141

Closed
ShiriMoran opened this issue Aug 17, 2023 · 2 comments
Closed

Is endpointElement really required #141

ShiriMoran opened this issue Aug 17, 2023 · 2 comments
Assignees
@adisos @ShiriMoran

Comments

@ShiriMoran
Copy link
Contributor 

          LGTM

We have to consider if EndpointElem is really required in the code, given that we already have VPCResourceIntf. This may allow us to reduce code duplication, as Node also embeds VPCResourceIntf. This can wait for now, but maybe open an issue for that...

Originally posted by @zivnevo in #136 (review)
This was referenced Aug 17, 2023
Closed
Merged
@zivnevo zivnevo mentioned this issue Aug 24, 2023
Closed
@adisos adisos self-assigned this Nov 12, 2023
@adisos
Copy link
Collaborator

adisos commented Nov 12, 2023

PR #222 has replaced EndpointElem with VPCResourceIntf on connectivity structures before grouping.
The EndpointElem type is currently used to represent grouped elements.

@ShiriMoran
Copy link
Contributor Author

The delta between EndpointElem and VPCResourceIntf is that the latter interface implements only Name() while the latter also implements UID(), Name(), ZoneName() and Kind(); the former functionality is non-relevant for grouped elements but required for VPC resources. Thus, the answer seems "yes", EndpointElem is required

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

@adisos adisos

@ShiriMoran ShiriMoran

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@adisos @ShiriMoran