Refactor getDataFromColumnPath logic
#2971
Conversation
* fix bug with long integers * simplify code and reduce duplication
julieg18
changed the title
getDataFromColumnPath logic
| @@ -147,7 +147,7 @@ describe('pickExperiments', () => { | |||
| description: '[exp-456]', | |||
| detail: `Created:${formatDate( | |||
| mockedExperiments[1].Created as string | |||
| )}, split:2.2000e+9, data/data.xml:22a1a29`, | |||
There was a problem hiding this comment.
Spotted this bug that was happening with long integers while working on the refactoring.
There was a problem hiding this comment.
This was actually intended (see #2813 (comment)).
There is a long tail of edge cases here. For example, why would we treat 2200043556.1 differently? What if we had 10000000000000000 or 10000000000000000.1? The JS numbers rabbit hole is deep.
There was a problem hiding this comment.
Oh, whoops! Must have missed that comment! The table has the full number so I assumed it was a bug. Will revert this change :)
There was a problem hiding this comment.
Do you think we should update the webview?
There was a problem hiding this comment.
Do you think we should update the webview?
Sure! Apologies if that's what you wanted originally, I misunderstood. When you say webview, are you referring to just the tooltips or the table as well? I'll open a pr that does both for now.
There was a problem hiding this comment.
I didn't even think about that until you replied. No need to apologise. It was probably me that missed something back in the original PR.
|
Code Climate has analyzed commit 5519cec and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.5% (0.0% change). View more on Code Climate. |
fix bug with long integersFollowup to comments in #2956