Bump versions #22
Bump versions #22
Conversation
There was a problem hiding this comment.
Hi, thanks for your efforts on this. Can you please undo all the formatting changes you put in? They make it hard to see what actual changes were necessary for this version upgrade. Ideally we can just support a wider list of dependency versions without requiring a breaking change, but it's not clear to me what parts of the code needed to be changed for this to work.
|
Should be ok now, can you please review? |
There was a problem hiding this comment.
Still seeing a lot of formatting changes. I'll add prettier separately so this isn't an issue in the future, but for this PR I'd prefer that it be kept to just the required changes. You should be able to modify the file and save without applying formatting in your editor. Besides the formatting, just one minor question about the counter increment and we should be good to go with what looks like a minor version bump (I'll do this post merge).
src/lineChunked.js
Outdated
| .attr('id', `d3-line-chunked-clip-${chunkName}-${counter++}`); | ||
| .attr('id', `d3-line-chunked-clip-${chunkName}-${counter}`); | ||
| } | ||
| counter += 1; |
There was a problem hiding this comment.
Why move the counter increment outside of the if statement?
There was a problem hiding this comment.
Good question, it's not that I have any preference personally but the eslint test enforces this. The tests fail otherwise.
| chunks.forEach((chunkName) => { | ||
| const chunkDef = chunkDefinitions[chunkName] || {}; | ||
| const evaluatedChunk = { | ||
| styles: Object.assign({}, |
There was a problem hiding this comment.
Why bother changing this to not use Object.assign?
There was a problem hiding this comment.
Again, no personal preference, but this is eslint enforced.
| const undefinedLineClass = '.d3-line-chunked-undefined'; | ||
| const definedPointClass = '.d3-line-chunked-defined-point'; | ||
|
|
||
| function getDocument() { |
|
Thanks for this, sorry for the outdated eslint config and lack of prettier |
2 participants
This brings all dependencies to recent versions, closing #21