Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Accept array of objects #18

Merged
merged 5 commits into from
Jun 18, 2018
Merged

Accept array of objects #18

merged 5 commits into from
Jun 18, 2018

Conversation

dubiousdavid
Copy link
Member

@dubiousdavid dubiousdavid commented Jun 12, 2018 •
edited
Loading

Fixes #16
Fixes #17

  • Accept array of objects (with column name auto-detection)
  • Properly quote cells, including escaping double-quotes
  • Some code cleanup, including removing some redundant calls.
  • Lots of unit-tests
  • Doc updates/improvements.

Tested locally on marketplace.

Required for: https://github.com/smartprocure/marketplace/pull/3832

@geosp
Copy link
Member

geosp commented Jun 13, 2018 •
edited
Loading

Warnings
⚠️

❗ This PR is BIG (+383 -137)
Please keep it below 500 net changes
Messages
📖

Your PR has no browser errors. Great job!
📖

No duti:fix npm script found. Not autofixing this PR.

Generated by 🚫 dangerJS

@coveralls
Copy link

coveralls commented Jun 13, 2018 •
edited
Loading

Coverage Status

Coverage increased (+3.1%) to 69.792% when pulling ca46ad0 on feature/16-array-of-objects into 762ee82 on master.

@dubiousdavid dubiousdavid self-assigned this Jun 13, 2018
geosp
geosp approved these changes Jun 18, 2018
View reviewed changes
Copy link
Member

@geosp geosp left a comment

Choose a reason for hiding this comment

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

Very nice.

@dubiousdavid dubiousdavid merged commit adc7295 into master Jun 18, 2018
@dubiousdavid dubiousdavid deleted the feature/16-array-of-objects branch June 18, 2018 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geosp geosp geosp approved these changes

@sadasant sadasant Awaiting requested review from sadasant

@daedalus28 daedalus28 Awaiting requested review from daedalus28

@dshishkov dshishkov Awaiting requested review from dshishkov

Assignees

@dubiousdavid dubiousdavid

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dubiousdavid @geosp @coveralls