filter out empty string #2323
filter out empty string #2323
Conversation
node/strings.go
Outdated
| import "strings" | ||
|
|
||
| //move from string.go. filter out empty string too. | ||
| func SplitAndTrim(s, sep, cutset string) []string { |
node/strings.go
Outdated
|
|
||
| import "strings" | ||
|
|
||
| //move from string.go. filter out empty string too. |
There was a problem hiding this comment.
The proper godoc should be in form of splitAndTrim ...
node/strings.go
Outdated
| @@ -2,7 +2,8 @@ package node | |||
|
|
|||
| import "strings" | |||
|
|
|||
| //move from string.go. filter out empty string too. | |||
| // SplitAndTrimEmpty is copied from common/string.go | |||
There was a problem hiding this comment.
Whats the point of saying that it is copied from common/string.go, we can just delete it from there as its only used here.
Codecov Report
@@ Coverage Diff @@
## develop #2323 +/- ##
===========================================
+ Coverage 60.88% 62.33% +1.44%
===========================================
Files 197 215 +18
Lines 16283 17617 +1334
===========================================
+ Hits 9914 10981 +1067
- Misses 5502 5726 +224
- Partials 867 910 +43
|
james-ray
force-pushed
the
master
branch
2 times, most recently
from
e4423ad
to
4819c93
Compare
|
Please don't keep amending the commits, we can squash it when merging. Constantly amending the commit makes it much harder to keep track of what the change between commits is :) It also hides our review comments which weren't addressed. |
|
@ValarDragon Ok,I got it. I will check more carefully. Retain all the commits. |
|
Sorry for the commits loss thing. |
libs/common/string.go
Outdated
| @@ -58,6 +58,23 @@ func SplitAndTrim(s, sep, cutset string) []string { | |||
| return spl | |||
| } | |||
|
|
|||
| // SplitAndTrimEmpty only returns non-empty strings | |||
| func SplitAndTrimEmpty(s, sep, cutset string) []string { | |||
node/node.go
Outdated
|
|
||
|
|
||
| // SplitAndTrimEmpty returns only non-empty strings | ||
| func SplitAndTrimEmpty(s, sep, cutset string) []string { |
There was a problem hiding this comment.
can you also make it private? splitAndTrimEmpty
There was a problem hiding this comment.
also we need to delete original cmn.SplitAndTrim and add a changelog entry saying "[common] SplitAndTrim was deleted"
There was a problem hiding this comment.
oh,really,can I delete it? I'll check if anywhere used it. I will leave a changelog.
CHANGELOG.md
Outdated
| @@ -38,6 +38,7 @@ IMPROVEMENTS: | |||
| - tweak params | |||
| - only process one block at a time to avoid starving | |||
| - [common] bit array functions which take in another parameter are now thread safe | |||
| - [common] SplitAndTrim was deleted | |||
There was a problem hiding this comment.
Its kind of confusing, but we put changelog updates for the next release in CHANGELOG_PENDING.md, could you move this there, and put it in the breaking changes section?
|
Looks good to me! Thank you for the PR! Just needs the changelog update moved to |
|
Thanks for your contribution 🖖 |
fix #2322