Fix type of EventInfo.PatchSet.Number #37
Conversation
Depending on the version of the events-log plugin and the version of Gerrit you can end up in a situation where some values of EventInfo.PatchSet.Number are a string and other are an integer. This change works around the problem by replacing the Number field with a custom type that can handle either strings or integers. Note, this does not change the underlying type of the Number field.
Codecov Report
@@ Coverage Diff @@
## master #37 +/- ##
=========================================
+ Coverage 17.45% 18.06% +0.6%
=========================================
Files 20 21 +1
Lines 1724 1738 +14
=========================================
+ Hits 301 314 +13
- Misses 1393 1394 +1
Partials 30 30
Continue to review full report at Codecov.
|
|
LGTM |
|
Thanks! I was really hoping there was a better way of handling this problem but I couldn't think of one. On the bright side, it's isolated to the plugin it seems and at least we'll have a way to deal with this if Gerrit itself has this kind of problem in the future. On a side note, any objections to me cutting a new release? |
|
I often follow the principle: Make it work, make it beautiful, make it fast.
Go for it. Just prepare the release notes to show what has changed. |
Will do, working on that now. |
| "strconv" | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
This .go file isn't gofmted.
| *n = Number(strconv.Itoa(number)) | ||
| return nil | ||
| } | ||
| return errors.New("Cannot convert data to number") |
There was a problem hiding this comment.
Style improvement suggestion, Go error strings should not be capitalized (unless beginning with proper nouns or acronyms).
See https://github.com/golang/go/wiki/CodeReviewComments#error-strings.
There was a problem hiding this comment.
Thanks for this.
Fixed it in 2572c0e
Thanks @shurcooL for pointing this out in #37 (review) Reference: https://github.com/golang/go/wiki/CodeReviewComments#error-strings
Depending on the version of the events-log plugin
and the version of Gerrit you can end up in a situation
where some values of EventInfo.PatchSet.Number are a
string and other are an integer. This change works around
the problem by replacing the Number field with a custom
type that can handle either strings or integers.
Note, this does not change the underlying type of the
Number field.