Number support for version color formatter #1615
Conversation
lib/color-formatters.js
Outdated
| @@ -7,11 +7,13 @@ | |||
| const moment = require('moment'); | |||
|
|
|||
| function version(version) { | |||
| let first = version[0]; | |||
| // The passed in version may or may not already be a string. See #1599. | |||
| const stringifiedVersion = '' + version; | |||
There was a problem hiding this comment.
This works! Another way I'm also fine with is like this:
version = '' + version;
There was a problem hiding this comment.
I code a lot in Java and we tend to avoid this, but I'm happy to use it here! 😄
There was a problem hiding this comment.
Yea… it's a matter of style. The language treats them as let variables, though if you want you can tell eslint not to let you reassign them. The advantage of reassigning them is that it prevents you from accidentally using the unmodified value, and I think reassigning them once at the top of the function is not so confusing. Though I could be convinced otherwise. 😁
lib/color-formatters.js
Outdated
| @@ -7,11 +7,13 @@ | |||
| const moment = require('moment'); | |||
|
|
|||
| function version(version) { | |||
| let first = version[0]; | |||
| // The passed in version may or may not already be a string. See #1599. | |||
There was a problem hiding this comment.
Since this can be inferred from the code itself, and tests will fail if this code is changed (which is really the best thing!) this comment isn't needed.
|
@RedSparr0w I thought the same initially, but actually you can't cover edge cases such as floating point numbers smaller than one without the string conversion. 0.1 would show as blue for instance, whereas we should actually treat it as a beta version. |
PyvesB
changed the title
This pull request is related to discussions in #1599.
Tests to cover cases where the version is provided as a numerical value were added. I thought of several different ways of tackling the implementation and went with one, but a more experienced Javascript reviewer may have some better ideas. 👍