Skip to content

feat: modify datafile managers to use datafile strings instead of objects #542

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.

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

Merged
merged 8 commits into from
Aug 5, 2020

Conversation

pthompson127
Copy link
Contributor

@pthompson127 pthompson127 commented Jul 28, 2020

Summary

  • Modified datafileManager and httpPollingDatafileManager to use datafile strings
  • Removed unneeded JSON parsing of response body

Test Plan

  • Modified existing datafile manager tests to account for new changes to datafile variable type

@coveralls
Copy link

coveralls commented Jul 28, 2020

Coverage Status

Coverage remained the same at 96.704% when pulling 0388cec on peter/datafile-manager-datafile-string into 8395f1d on master.

@pthompson127 pthompson127 requested a review from mjc1283 July 29, 2020 22:56
@pthompson127 pthompson127 removed their assignment Jul 29, 2020
Copy link
Contributor

@mjc1283 mjc1283 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . Small changes:

  • Please add an entry in the Unreleased section of CHANGELOG.md describing this change
  • There appear to be some formatting issues, please run npm run lint and commit those changes.

@pthompson127 pthompson127 requested a review from mjc1283 July 30, 2020 18:25
@pthompson127 pthompson127 requested a review from mjc1283 July 31, 2020 18:38
@pthompson127 pthompson127 removed their assignment Aug 3, 2020
@pthompson127 pthompson127 merged commit 9cea0b6 into master Aug 5, 2020
@pthompson127 pthompson127 deleted the peter/datafile-manager-datafile-string branch August 5, 2020 00:00
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.

3 participants