Remove the channel Iterator method from ActiavePipelines #175
Conversation
Codecov Report
@@ Coverage Diff @@
## master #175 +/- ##
==========================================
+ Coverage 66.78% 66.82% +0.03%
==========================================
Files 33 33
Lines 2628 2607 -21
==========================================
- Hits 1755 1742 -13
+ Misses 650 642 -8
Partials 223 223
Continue to review full report at Codecov.
|
Skarlso
added
needs review
and removed
Needs Work
handlers/pipeline.go
Outdated
| @@ -97,7 +97,7 @@ func PipelineGetAll(c echo.Context) error { | |||
| var pipelines []gaia.Pipeline | |||
|
|
|||
| // Get all active pipelines | |||
| for pipeline := range pipeline.GlobalActivePipelines.Iter() { | |||
| for _, pipeline := range pipeline.GlobalActivePipelines.Iter() { | |||
There was a problem hiding this comment.
We get the full list of pipelines already. No need to copy that. Just return it.
handlers/pipeline.go
Outdated
| @@ -438,7 +434,7 @@ func PipelineGetAllWithLatestRun(c echo.Context) error { | |||
| // Get all active pipelines | |||
| storeService, _ := services.StorageService() | |||
| var pipelines []gaia.Pipeline | |||
| for pipeline := range pipeline.GlobalActivePipelines.Iter() { | |||
| for _, pipeline := range pipeline.GlobalActivePipelines.Iter() { | |||
There was a problem hiding this comment.
We already have the list of pipelines. I think there is no need to copy that.
workers/pipeline/create_pipeline.go
Outdated
| @@ -178,7 +178,7 @@ func ValidatePipelineName(pName string) error { | |||
|
|
|||
| // Check if pipeline name is already in use. | |||
| alreadyInUse := false | |||
| for activePipeline := range GlobalActivePipelines.Iter() { | |||
| for _, activePipeline := range GlobalActivePipelines.Iter() { | |||
| if strings.ToLower(s) == strings.ToLower(activePipeline.Name) { | |||
| alreadyInUse = true | |||
There was a problem hiding this comment.
if we found it we can break here
workers/pipeline/git.go
Outdated
| @@ -190,7 +190,7 @@ func updateAllCurrentPipelines() { | |||
| var allPipelines []gaia.Pipeline | |||
| var wg sync.WaitGroup | |||
| sem := make(chan int, 4) | |||
| for pipeline := range GlobalActivePipelines.Iter() { | |||
| for _, pipeline := range GlobalActivePipelines.Iter() { | |||
There was a problem hiding this comment.
No need to copy the list again
workers/pipeline/pipeline.go
Outdated
| close(c) | ||
| }() | ||
|
|
||
| func (ap *ActivePipelines) Iter() []gaia.Pipeline { |
There was a problem hiding this comment.
Iter sounds like you get an iterator back. Maybe GetAll or ToSlice is better?
There was a problem hiding this comment.
@michelvocks I didn't wanted to change the whole thing at once... I thought I'll keep the name for now, to see if anything breaks.
But something I didn't realise is that I'm already giving back a list. Why am I going over it again in some situations. :D Made no sense. thanks for spotting that.
I'll rename it. :)
Closes #163