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.

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

Separate epjs related code into it's own repo (this repo) #1

Open
RobStallion opened this issue Jul 24, 2017 · 6 comments
Open

Separate epjs related code into it's own repo (this repo) #1

RobStallion opened this issue Jul 24, 2017 · 6 comments

Comments

@RobStallion
Copy link
Contributor

The epjs related code was originally built into the healthlocker app but is now going to be put into it's own repository and required into the HL app when needed.

See here for more info on this.

@RobStallion RobStallion self-assigned this Jul 24, 2017
RobStallion added a commit that referenced this issue Jul 29, 2017
RobStallion added a commit that referenced this issue Jul 29, 2017
@nelsonic
Copy link
Contributor

nelsonic commented Aug 1, 2017

@RobStallion what error were you seeing when you created this file:
https://github.com/healthlocker/epjs_app/blob/b224bc67d6f3925adb6a267fa90726e70be96ddd/lib/epjs_app/encoder.ex
... ?
I'm getting a compilation error because of it ... devinus/poison#109
There is a warning for this in: https://travis-ci.org/healthlocker/epjs_app/builds/258521507#L719

Would be really useful to have a comment in the file explaining why the protocol is being re-implemented.

Let me know when you have time to pair on this in the morning. 👍

@RobStallion
Copy link
Contributor Author

@nelsonic. I was getting the following error...

unable to encode value: {nil, “some_data"}

I did a bit of googling and it seemed to be an encoding issue with ecto and poison. The solution I used to fix it can be found here

@nelsonic
Copy link
Contributor

nelsonic commented Aug 2, 2017

@RobStallion thanks for reply.
Where is the test for this code?
(so that I can re-factor it and keep it working without throwing a warning and subsequent compilation error...)

Related: It appears that only two tests are running on Travis:
https://travis-ci.org/healthlocker/epjs_app/builds/258521507#L738
image
(yet there are quite a few tests in the project... any idea why they aren't all running on Travis-CI?)

@RobStallion
Copy link
Contributor Author

RobStallion commented Aug 2, 2017

@nelsonic I must have missed the test for this 😓.

With regards to why only 2 tests are running in travis, I am not sure why this is the case.

Will look into both points you raised now, starting with trying to test the function which fixed the encoding error we were getting.

@nelsonic
Copy link
Contributor

nelsonic commented Aug 2, 2017

@RobStallion sweet. here at the "Big" monitor in case you want to pair on any of it.
Otherwise I will continue on HL (main app) deploy. 📦

@RobStallion
Copy link
Contributor Author

@nelsonic. Not sure exactly what happened with the tests on travis there. Just took a screen shot of the latest build from travis

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants