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

98 grouping with self loops #152

Merged
merged 55 commits into from
Sep 28, 2023
Merged

98 grouping with self loops #152

merged 55 commits into from
Sep 28, 2023

Conversation

ShiriMoran
Copy link
Contributor

solves #98

@ShiriMoran
So that grouping can be reused by subnetConnectivity
ee600cf
@ShiriMoran
VPCsubnetConnectivity computed and stored on EndpointElem instead of …
43e8d41 
…string so that (among others) grouping can be reused
@ShiriMoran
Merge remote-tracking branch 'origin/main'
c3f5519 
# Conflicts:
#	pkg/vpcmodel/grouping.go
#	pkg/vpcmodel/subnetsConnectivity.go
@ShiriMoran
Merge remote-tracking branch 'origin/main'
2eb1ef1
@ShiriMoran
fix comment (should have been part of a previous branch)
df04274
@ShiriMoran
extracting code to a function to be reused
e202533
@ShiriMoran
grouping with self loops - the first stages
b9f3d3f
@ShiriMoran
commit before context switching to #144
b97263d
@ShiriMoran
Merge remote-tracking branch 'origin/main' into 138_grouping_with_sel…
abb8c03 
…f_loops
@ShiriMoran
code for computing delta between GroupedConnLine
fe16a2d
@ShiriMoran
mergeGroups computed
7df7c1d
@ShiriMoran
merge groups to be merged
237f70b
@ShiriMoran
vsis should be grouped only if from the same subnet
82d7c34
@ShiriMoran
update testing
af7e0ab
@ShiriMoran
fixed comment
6897003
@ShiriMoran
lint comments #1
24e903e
@ShiriMoran
lint comments #2
2deebf5
@ShiriMoran
lint comments #3
be56d4d
@ShiriMoran ShiriMoran force-pushed the 98_grouping_with_self_loops branch from 8465af1 to be56d4d Compare September 5, 2023 12:48
@ShiriMoran ShiriMoran requested review from zivnevo and adisos September 5, 2023 13:18
adisos
adisos reviewed Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@adisos adisos left a comment

Choose a reason for hiding this comment

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

Added a few initial comments

pkg/vpcmodel/grouping.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Show resolved Hide resolved
pkg/vpcmodel/grouping_test.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
adisos
adisos reviewed Sep 26, 2023
View reviewed changes
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
adisos
adisos reviewed Sep 26, 2023
View reviewed changes
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
adisos
adisos reviewed Sep 27, 2023
View reviewed changes
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
pkg/vpcmodel/groupingSelfLoop.go Outdated Show resolved Hide resolved
adisos
adisos reviewed Sep 27, 2023
View reviewed changes
Copy link
Collaborator

@adisos adisos left a comment

Choose a reason for hiding this comment

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

General comment: please add documentation for individual functions about their input args and return values, and what they represent, especially for non-trivial functions (such as deltaBetweenGroupedConnLines , setMinusSet... ).

@ShiriMoran ShiriMoran merged commit 4af4337 into main Sep 28, 2023
@ShiriMoran ShiriMoran deleted the 98_grouping_with_self_loops branch September 28, 2023 05:42
@ShiriMoran ShiriMoran mentioned this pull request Sep 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adisos adisos adisos approved these changes

@zivnevo zivnevo Awaiting requested review from zivnevo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ShiriMoran @adisos