Created an error value, in case the for loop fails with index not found. #206
Conversation
Skarlso
changed the title
Codecov Report
@@ Coverage Diff @@
## master #206 +/- ##
==========================================
- Coverage 62.2% 62.17% -0.04%
==========================================
Files 48 48
Lines 3934 3944 +10
==========================================
+ Hits 2447 2452 +5
- Misses 1082 1085 +3
- Partials 405 407 +2
Continue to review full report at Codecov.
|
workers/pipeline/pipeline.go
Outdated
| ap.Lock() | ||
| defer ap.Unlock() | ||
|
|
||
| l := len(ap.Pipelines) | ||
| if index >= l || index+1 > l { |
There was a problem hiding this comment.
| if index >= l || index+1 > l { | |
| if index >= l || index < 0 { |
There was a problem hiding this comment.
Hey. :)
So, yeah, I left out 0 that's a good call. However, I'll tell you why index+1 > l needs to be there. :D It's a mind bender so...
This is the line we are protecting:
ap.Pipelines = append(ap.Pipelines[:index], ap.Pipelines[index+1:]...)
if l == 0:
The first part of append will fail.
if l == 1 and index = 1:
The first part will work because slice postfix is not inclusive.
But the second part, the index+1 will fail because that one is inclusive. And 2 is not a valid index to look from.
|
@Skarlso overall LGTM. Only these two issues |
Co-Authored-By: Michel Vocks <michelvocks@gmail.com>
|
Closes #204. |
No description provided.