nparashuram / cordova-plugin-browsersync

BrowserSync Plugin for Cordova
110 stars 69 forks source link

Use common npm sub-module for the utility functions needed for browsersync on cordova #12

Closed asselin closed 8 years ago

asselin commented 8 years ago

I recently factored out the utility functions needed to enable browersync on cordova into a separate github repo and npm module (https://github.com/PointSource/cordova-browsersync-primitives). These are the functions to add CSP, fix ATS, etc. I did this because I need these functions for a project I'm working on, and I know there are also 2 existing projects that also use them (https://github.com/nparashuram/cordova-plugin-browsersync and https://github.com/Microsoft/TACO).

The purpose of this issue is to suggest that we consolidate around 1 implementation of these utility functions that we can all pull in as an npm submodule.

@axemclion, what's your opinion of converting over to use a common implementation?

axemclion commented 8 years ago

@asselin +1. This module was written before TACO has the browser-sync implementation. We also have another implementation https://github.com/omefire/cordova-plugin-livereload/ that is pretty similar.

I think we should deprecate the others and have just one implementation of browser sync.

/cc @omefire

asselin commented 8 years ago

@axemclion According to the README on cordova-plugin-livereload, it's been deprecated and folded into TACO. I'm going to work on a PR for TACO for the issue I opened there (https://github.com/Microsoft/TACO/issues/258). Do you want me to submit one for this issue too?

omefire commented 8 years ago

+1

asselin commented 8 years ago

Starting work on this today

asselin commented 8 years ago

@axemclion How much should I worry about the https://github.com/nparashuram/virtual-device-wall project relying on this project? Is it still maintained? Do you care if updates to this project break that one?

axemclion commented 8 years ago

@asselin I still do maintain that, but if you do break, I think I can fix it.

asselin commented 8 years ago

@axemclion Have you had a chance to review the PR? If there's anything blocking merging it, let me know what I can do to help. Thanks!

axemclion commented 8 years ago

Closing this issue since this was merged.