apache / cordova-paramedic

Apache Cordova - Paramedic
https://cordova.apache.org/
Apache License 2.0
36 stars 53 forks source link

Extract classes from paramedic.js (App, SauceLabs) #86

Closed janpio closed 5 years ago

janpio commented 5 years ago

This PR extracts two big parts of paramedic.js into their own classes: ParamedicApp which handles the creation of the app, and ParamedicSauceLabs which does all the Sauce Labs things.

Along the way I renamed ParamedicLog to ParamedicLogCollector, resorted some requires and expanded our npm scripts list to enable better testing of paramedic.

The easiest way to review this PR is probably by looking at the individual PRs that show how I first extracted the code without changing it, then fixed all the issues that popped up because of the new data locations etc.

closes #74

purplecabbage commented 5 years ago

Sure ... I mean, paramedic is now a hospital. Not crazy about renaming things just to rename them, lib/ParamedicLog.js → lib/ParamedicLogCollector.js Also, this code in general is over objectified ... a functional approach would make much more sense given that there is only ever 1 of each of these object ... but that's just my preference.

janpio commented 5 years ago

Oh yes it is.

For the object stuff... this way was just the most simple way to extract some of the code into its own file, following the existing "patterns". Just understanding what method belongs where took me quite some time tbh. That was important and useful to make this more debuggable, but maybe one day someone comes along and refactors it all. It sure won't be me though 🙈

(The Log=>LogCollector represents what the code actually does by the way - it collects logs from the devices, and doesn't really log itself)