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

sotw code cleanup #438

Closed
wants to merge 37 commits into from
Closed

sotw code cleanup #438

wants to merge 37 commits into from

Conversation

alecholmez
Copy link
Contributor

  • moves server unit tests to assert
  • removes code duplication inside of the watches

Signed-off-by: Alec Holmes <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
@alecholmez
Copy link
Contributor Author

alecholmez commented May 25, 2021

@snowp I ported the logic over from delta to sotw so we cleanup the existing code a bit 😄 .

In another PR we can address the following bugs: #431 #399

But just wanted to get us in a place where those will be easy to implement

@snowp
Copy link
Contributor

snowp commented May 26, 2021

There is a slight perf concern (additional goroutines per stream) with making this change, I wonder if we want to wait until we have mem/speed benchmarks until we make a change like this to SotW since it's in active use?

@alecholmez
Copy link
Contributor Author

@snowp I'm not opposed to that. go routines are relatively lightweight but I think it's a good idea to measure the current performance first.

go-control-plane(Azure Pipelines) and others added 21 commits June 21, 2021 11:36
…5bbb

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…a1bf

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…ab39

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…d0e3

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…0bf8

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…b199

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
fixed integration tests for ads
…fe31

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…ab58

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…dbc1

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…e43f

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…0f7e

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…1410

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…7b08

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…73bc

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…0083

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…215d

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…d40e

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…bf6f

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
go-control-plane(Azure Pipelines) and others added 13 commits June 21, 2021 11:37
…73e5

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…5067

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…c613

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…4666

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…1216

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…362b

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…b554

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…7466

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
…d623

Signed-off-by: go-control-plane(Azure Pipelines) <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
Signed-off-by: Alec Holmes <[email protected]>
@alecholmez alecholmez closed this Jun 21, 2021
@alecholmez alecholmez deleted the sotw-dedupe branch June 21, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants