Skip to content

Conversation

Jarvis1010
Copy link

@Jarvis1010 Jarvis1010 commented Feb 15, 2018

I just merged with the latest commits, I believe this should be correct now

Copy link
Collaborator

@abdennour abdennour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Jarvis1010 This is a great feature. CSVLink will be the main component and it will ease the maintenance of CSVDownload component. I have only one concern. Indeed, any file under ""cdn/" folder should appear in the pull-request since CDN will be generated just before shippment to NPM

@mccabemj
Copy link
Collaborator

@Jarvis1010 can you update this PR to fix/change failing unit tests and conflicts? I think it would be a good thing to get merged

@Jarvis1010 Jarvis1010 force-pushed the master branch 2 times, most recently from fba7ac1 to ce74cd1 Compare October 18, 2018 22:15
@coveralls
Copy link

coveralls commented Oct 18, 2018

Coverage Status

Coverage decreased (-0.2%) to 83.505% when pulling 04b149a on Jarvis1010:master into 4705a69 on abdennour:master.

@Jarvis1010
Copy link
Author

Updated and CI working but coverage has decreased

@AdamLiechty
Copy link

AdamLiechty commented Dec 17, 2018

@abdennour @mccabemj I have resolved the merge conflicts in this PR here: #129

@AdamLiechty
Copy link

#137 has these changes, along with fixes to unit tests, and without spurious style changes

@mccabemj mccabemj closed this Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants