Coding style guide modifications #907
Conversation
| icd10Code: DS.attr('string') | ||
| icd10Code: DS.attr('string'), | ||
| // Associations | ||
| patient: DS.belongsTo('patient'), |
There was a problem hiding this comment.
Trailing comma caused the eslint test to fail
There was a problem hiding this comment.
yes! i run the tests locally and missed that. my bad!
There was a problem hiding this comment.
No problem, just saw that the test output was a little noisy since I need to clean up a few validators in the tests and wasn't sure if you could find it in there. 🍹
|
@hsorellana thanks for the PR! It looks like the tests are failing. Are you still working on this PR? |
|
sorry the inactivity, @jkleinsc , i'll keep working on this PR, i had some troubles with the tests, every tool i use to run them, gives me different results, let's see how it goes now ;) |
|
@hsorellana I've been working on some other stuff this week, but I have been doing a fair amount of work with the tests. If there is anything I can do to help get things working let me know. My email is available on my github profile or ping me in the slack channel if you prefer that. |
|
@hsorellana One additional note. I do not want to speak for @jkleinsc but you might want to think about breaking these up in the smaller PRs. It would keep the already passing ones from getting held up by the ones that may be having issues and would probably be easier for reviewers to notice anything they might have concerns about. Thanks for all the work you have put into to these already. |
|
@mkly hey Mike! Thanks for the feedback, and probably i'll messaging you through Slack when i'm in doubt with something! Probably tests will be the main topic of my questions, i'm in my senior year at college and i've readying about tests just now, lame, i know jeje |
|
@hsorellana no worries about the delay. As @mkly noted, smaller PRs would be better and in the case of refactoring for formatting reasons I think maybe it would be better to do one file per PR. It makes it easier on your side to make sure tests pass and easier on my side to see what has been changed |
|
@jkleinsc Hey John! I took your advice, and @mkly 's too, and divided into smaller PRs. I have two open PRs, one for the patient model, and another one with several model modifications. I did this because they are just reordering of the model attributes, and i made sure that every test passed locally before committing it to my fork repo. |
|
@hsorellana thanks for breaking up items to smaller PRs. I am going to close this issue in favor of the smaller PRs. |
Re-write, on this instance, of the models on the application.
cc @HospitalRun/core-maintainers