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

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

Merged
merged 11 commits into from
Mar 27, 2019

Conversation

janpio
Copy link
Member

@janpio janpio commented Dec 5, 2018

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

@project-bot project-bot bot added this to 🐣 New PR / Untriaged in Apache Cordova: Tooling Pull Requests Dec 5, 2018
@janpio janpio changed the title [WIP] split paramedicjs [WIP] Extract classes from paramedic.js Dec 5, 2018
@janpio janpio changed the title [WIP] Extract classes from paramedic.js Extract classes from paramedic.js Dec 5, 2018
@janpio janpio changed the title Extract classes from paramedic.js Extract classes from paramedic.js (App, SauceLabs) Dec 5, 2018
@purplecabbage
Copy link
Contributor

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.

Apache Cordova: Tooling Pull Requests automation moved this from 🐣 New PR / Untriaged to ✅ Approved, waiting for Merge Dec 5, 2018
@janpio
Copy link
Member Author

janpio commented Dec 5, 2018

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)

@janpio janpio merged commit a1e2493 into master Mar 27, 2019
Apache Cordova: Tooling Pull Requests automation moved this from ✅ Approved, waiting for Merge to 🏆 Merged, waiting for Release Mar 27, 2019
@janpio janpio deleted the janpio-split_paramedicjs branch March 27, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Apache Cordova: Tooling Pull Requests
🏆 Merged, waiting for Release
Development

Successfully merging this pull request may close these issues.

Extract Sauce Labs Code
2 participants