cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikhil Khandelwal <nikhi...@microsoft.com>
Subject RE: [Discuss] Cordova-common release
Date Tue, 20 Oct 2015 21:23:24 GMT
+1 to publishing cordova-common to npm. 

-Nikhil

-----Original Message-----
From: Steven Gill [mailto:stevengill97@gmail.com] 
Sent: Tuesday, October 20, 2015 2:20 PM
To: dev@cordova.apache.org
Subject: Re: [Discuss] Cordova-common release

I want to revisit this.

So cordova-android has a dependency now on cordova-common. It is a bundledDependency so when
we generate a tar to release cordova-android, it will be included. It will also be in the
cordova-android package that gets downloaded with cordova platform add.

This is fine for released work, but more annoying for development. I need to npm link cordova-common
into cordova-android (and soon every platform which implements common platformAPI). We could
check in cordova-common into cordova-android but that isn't a great solution either.

I agree that we should be going towards smaller modules and not having a case of cordovaLib1,
cordovaLib2, etc. I think this is still going to be a work in progress and will take some
time.

For the interim, I recommend we publish cordova-common. Of course, continue to add it as a
bundledDependency so users don't need to npm install it with released packages.

On Wed, Sep 30, 2015 at 7:24 AM, Vladimir Kotikov (Akvelon) < v-vlkoti@microsoft.com>
wrote:

> > I still do not understand what are you trying to solve by having all
> that content published as big blob.
> Code deduplication is the main reason. All the things from 
> 'cordova-common' will be used by platforms intensively, so we need to 
> share this code and keep it separately from LIB to share easily. 
> Publishing is basically doesn't required for this, and bundling 
> 'cordova-common' into LIB is enough for this purpose.
>
> Another reason was that third-party tool might want to use some of 
> this functionality (like your example with ConfigParser), so we need 
> to have this package on NPM to allow them to get it. For this case I 
> now do agree with you that separate packages for ConfigParser, 
> PluginInfo and other stuff looks better than putting it into one big package.
>
> -
> Best regards, Vladimir
>
>
> -----Original Message-----
> From: Carlos Santana [mailto:csantana23@gmail.com]
> Sent: Wednesday, September 30, 2015 2:07 PM
> To: dev@cordova.apache.org
> Subject: Re: [Discuss] Cordova-common release
>
> Yes temporary, maybe we can discuss some more in F2F
>
> I still do not understand what are you trying to solve by having all 
> that content published as big blob.
>
> If the packages are only for Cordova components to depend on then we 
> control the release and we can include them easily.
>
> If the code is to be share by third party or anyone out there then it 
> make sense to put in npm.
>
> One concrete example is cordova-configparser, Our IBM tool is using it 
> in our own models code so today we taking a copy, if it's available 
> thru npm then we can stated as a dependency and manage it as a npm 
> package vs a loosely node module js file
>
> Maybe not all classes need to be converted to npm packages maybe it 
> can be some cordova-configparser cordova-utils cordova-helper
>
> Also do some refactoring and dependency cleaning, I saw a node module 
> dependeding on underscore and the file only had one simple call to 
> _.find()
>
> We were going to use that module, but then decided not to since it 
> depended on underscore for a simple thing, this creates legal 
> clearance work and more dependencies we need to include in our product 
> making our download larger.
>
> And yes, yes we bundle all our dependencies when we go into production.
>
> - Carlos
> Sent from my iPhone
>
> > On Sep 30, 2015, at 5:27 AM, Vladimir Kotikov (Akvelon) <
> v-vlkoti@microsoft.com> wrote:
> >
> > Including cordova-common as bundled dependency should be enough in 
> > our
> case. Changes to coho are submitted already [1], so we only need to 
> update package.json for cordova-lib.
> >
> > Personally  for me bundling looks like a temporary workaround before 
> > we
> get all those partials of 'common' published to npm, am I right?
> >
> > [1]
> > https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgit
> > hu
> > b.com%2fapache%2fcordova-coho%2fpull%2f94&data=01%7c01%7cv-vlkoti%40
> > 06
> > 4d.mgd.microsoft.com%7cf9eefe800e4c44c0540308d2c9874d65%7c72f988bf86
> > f1 
> > 41af91ab2d7cd011db47%7c1&sdata=HpV5fWkx6nUZUU%2fT4qVVo0QZiGM7%2fm%2b
> > do
> > 9WX4V4JfT0%3d
> >
> > -
> > Best regards, Vladimir.
> >
> > -----Original Message-----
> > From: Carlos Santana [mailto:csantana23@gmail.com]
> > Sent: Tuesday, September 29, 2015 9:15 PM
> > To: dev@cordova.apache.org
> > Subject: Re: [Discuss] Cordova-common release
> >
> > Do we need to absolutely publish this to npm?
> >
> > Can we just include the dependency in the platform as a bundle
> dependency?
> >
> > We just need to update coho to npm install/link the cordoba-common 
> > package when doing a release of what ever component need it? (i.e.
> > cordova-android)
> >
> > Will this get you what you want? Why does it absolutely need to be 
> > in
> npm registry?
> >
> > I really don't think will be a good idea to publish two npm packages
> "cordova-lib" and "cordova-common"
> >
> > Sorry if I'm being a pain in the ass, maybe I'm something obvious 
> > here
> >
> >
> >> On Tue, Sep 29, 2015 at 1:34 PM Steven Gill 
> >> <stevengill97@gmail.com>
> wrote:
> >>
> >> Sounds good. Let's move forward
> >> On Sep 29, 2015 10:21 AM, "Nikhil Khandelwal"
> >> <nikhilkh@microsoft.com>
> >> wrote:
> >>
> >>> +1. I understand the value of Carlos' proposal, but in the spirit 
> >>> +of
> >>> moving forward with this which is fairly complicated refactor 
> >>> involving multiple releases and repos, I would like us to make 
> >>> progress on this
> >> soon
> >>> and not add significant scope to this effort.
> >>>
> >>>
> >>> -Nikhil
> >>>
> >>> -----Original Message-----
> >>> From: Sergey Grebnov (Akvelon) [mailto:v-segreb@microsoft.com]
> >>> Sent: Tuesday, September 29, 2015 1:34 AM
> >>> To: dev@cordova.apache.org
> >>> Subject: RE: [Discuss] Cordova-common release
> >>>
> >>> +1
> >>>
> >>> -----Original Message-----
> >>> From: Vladimir Kotikov (Akvelon) [mailto:v-vlkoti@microsoft.com]
> >>> Sent: Tuesday, September 29, 2015 11:27 AM
> >>> To: dev@cordova.apache.org
> >>> Subject: RE: [Discuss] Cordova-common release
> >>>
> >>> Agree with you, guys.
> >>>
> >>> Unfortunately, the underlying modules in `cordova-common` are not 
> >>> really atomic, since they depending on each other. For example 
> >>> ConfigParser requires `xmlHelpers`, `events` and `CordovaError` as 
> >>> a
> dependencies.
> >>> Reworking them to be truly separated might be sort of problematic, 
> >>> especially in context of message logging (as they use shared event
> >> emitter
> >>> to log output to console).
> >>>
> >>> So I still propose is to release `common` module as-is and then 
> >>> gradually move inner modules out to separate packages.
> >>>
> >>> -
> >>> Best regards, Vladimir.
> >>>
> >>> -----Original Message-----
> >>> From: Carlos Santana [mailto:csantana23@gmail.com]
> >>> Sent: Friday, September 25, 2015 7:33 PM
> >>> To: dev@cordova.apache.org
> >>> Subject: Re: [Discuss] Cordova-common release
> >>>
> >>> Sorry a typo
> >>> to use "bundleDependencies" you will have a node_modules/ 
> >>> directory directly under "common/node_modules/cordova-error/"
> >>>
> >>> and the the small modules (i.e. cordoba-util, cordova-plugin-info,
> >>> etc..) will be located there.
> >>>
> >>> then have explicit ignores for the dependencies you don't want to 
> >>> be source control like npm [2]
> >>>
> >>> [2]:
> >> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgi
> >> th
> >> u
> >> b.com%2fnpm%2fnpm%2fblob%2fmaster%2f.gitignore%23L24&data=01%7c01%7
> >> cv
> >> -
> >> vlkoti%40064d.mgd.microsoft.com%7c73b4ff38f0fe41e1f18608d2c5c70e0f%
> >> 7c
> >> 7
> >> 2f988bf86f141af91ab2d7cd011db47%7c1&sdata=tU%2bFHDUJZXzXnbG%2fUP7AY
> >> 4q
> >> E
> >> CnvsbnsJ%2bvEriJvqYcU%3d
> >>>
> >>>
> >>> On Fri, Sep 25, 2015 at 12:24 PM Carlos Santana 
> >>> <csantana23@gmail.com>
> >>> wrote:
> >>>
> >>>> Yes after reviewing the changes, I understood the purpose of the 
> >>>> code that you seperated to avoid duplicate code between the other 
> >>>> top level modules (i.e. platforms, lib, cli)
> >>>>
> >>>> I still think small modules is the way to go.
> >>>>
> >>>> Don't let process, bureaucracy, and ceremony hamper the 
> >>>> engineering process and the consumer UX using this modules.  
> >>>> (yeah that came out from the mouth of an IBMer ;-p)
> >>>>
> >>>> Yes, I'm not blind, I understand the voting, the releasing, the 
> >>>> packaging the publish steps
> >>>>
> >>>> The way I look at it no matter what you do you are not going to 
> >>>> make it easier by having one "common" package.
> >>>>
> >>>> Let's say you just need to update a file to fix a bug from Error, 
> >>>> well you need to test, vote, release, "common"
> >>>> Next you want to fix a bug in configparser, guess what you need 
> >>>> to tests, vote, release "common" again But if only config parser 
> >>>> changed all the rest of the code in "common"
> >>>> needs to be tested and release, and consumer will need to pick a 
> >>>> new common for only a small bug fix in a portion of "common"
> >>>>
> >>>> Basically that's what we have today, the way I see it you are 
> >>>> just creating two libs "lib" and "lib2"
> >>>>
> >>>> With large number of small modules, lets say we create 8 now, 
> >>>> maybe only 2 change most of the time, and the other 5 are so 
> >>>> basic and small that they never change and stay at version 1.0.0 for
 long time.
> >>>>
> >>>> I think for this small modules, I don't think we have to do the 
> >>>> blog post, twitter things, those I will continue to have for the 
> >>>> large components (cli, platforms, plugins)
> >>>>
> >>>> Also the side effect I would like to see, is clean APIs edges, 
> >>>> each small module provides an API, it contain tests to that API, 
> >>>> and lib contain integration tests as a whole.
> >>>>
> >>>> Maybe the compromise for now, to move forward let's break the 
> >>>> functions into "npm packages" inside this "common" where each sub 
> >>>> directory inside common is a npm package with a single entry 
> >>>> point
> >>>> (index.js) and common package.json have them as 
> >>>> "bundleDependencies", similar way as npm does [1]
> >>>>
> >>>> the transition will be for consumers for their dependencies and 
> >>>> the way they consume the API
> >>>> dependencies: {
> >>>>   cordova-common: "1.0.0"
> >>>> }
> >>>> cordova-common only expose one index.js with the APIs to the 
> >>>> other modules
> >>>>
> >>>> var cdvUtil              = require("cordova-common").cordova-util
> >>>> cdvPluginInfo          =
> require("cordova-common").cordova-plugin-info,
> >>>> cdvError                  = require("cordova-common").cordova-error,
> >>>> cdvConfigParser     = require("cordova-common").cordova-config-parser,
> >>>> cdvConfigChanges =
> >>>> require("cordova-common").rcordova-config-changes);
> >>>>
> >>>> then it can be easier transition if we want to change later, 
> >>>> nothing much on our part since we already have the npm packages 
> >>>> implemented it's a matter if we want to make them available on 
> >>>> npm or
> not.
> >>>>
> >>>> [1]:
> >>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2f
> >>>> g
> >>>> ithu
> >>>> b.com%2fnpm%2fnpm%2fblob%2fmaster%2fpackage.json%23L137&data=01%7
> >>>> c
> >>>> 01%7
> >>>> cv-vlkoti%40064d.mgd.microsoft.com%7c73b4ff38f0fe41e1f18608d2c5c7
> >>>> 0
> >>>> e0f%
> >>>> 7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=tUdBPvbE8oC3DPubmVx5
> >>>> 0
> >>>> QKD2
> >>>> CLfoxrVgj%2ftTxTrMJ8%3d
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On Fri, Sep 25, 2015 at 11:40 AM Sergey Grebnov (Akvelon) < 
> >>>> v-segreb@microsoft.com> wrote:
> >>>>
> >>>>> I tend to agree w/ Carlos here, but from practical side it might

> >>>>> be very hard to maintain and release such a granular modules, 
> >>>>> for example,
> >>>>> -  cordova-error has been updated ->Vote -> update 
> >>>>> cordova-config-parser
> >>>>> + Vote-> update + Vote other depended modules
> >>>>> - now we want to add some new feature: modules are very granular

> >>>>> so we should introduce a new module
> >>>>>
> >>>>> But I totally love and support Carlos idea regarding grouping 
> >>>>> meaningful/independent logic in modules, this is how software 
> >>>>> must be designed.
> >>>>>
> >>>>> I personally think about this new module as some sort of core 
> >>>>> Cordova functionality and high level classes which could be used

> >>>>> by cordova-lib/cli and platforms -unified CordovaError, events 
> >>>>> (output tracing, etc), working with config file, superspawn, etc
> >>>>>
> >>>>> Thx!
> >>>>> Sergey
> >>>>> -----Original Message-----
> >>>>> From: Carlos Santana [mailto:csantana23@gmail.com]
> >>>>> Sent: Thursday, September 24, 2015 6:31 PM
> >>>>> To: dev@cordova.apache.org
> >>>>> Subject: Re: [Discuss] Cordova-common release
> >>>>>
> >>>>> Sorry if I take my inner purist theoretical developer out for a

> >>>>> minute :-)
> >>>>>
> >>>>> The point of having a "node module" it should be explicit and 
> >>>>> small, meaning that should be easy to name in a way that 
> >>>>> describes what it is or do.
> >>>>>
> >>>>> Take into account that "node module" is not the same as a "npm
> >> package"
> >>>>>
> >>>>> Having 2 npm packages on the npm registry "cordova-common" and 
> >>>>> "cordova-lib" to the simple eye would look like duplicate 
> >>>>> packages, and then will need to answer multiple times "What is 
> >>>>> the difference between lib and common?"
> >>>>>
> >>>>> Why not have more smaller explicit npm packages instead?
> >>>>>
> >>>>> cordova-util
> >>>>> cordova-plugin-info
> >>>>> cordova-error
> >>>>> cordova-config-parser
> >>>>> cordova-config-changes
> >>>>>
> >>>>> each one with a index.js exposing APIs
> >>>>>
> >>>>> Then the programing model becomes something like this:
> >>>>> var cdvUtil              = require('cordova-util'),
> >>>>> cdvPluginInfo          = require('cordova-plugin-info'),
> >>>>> cdvError                  = require('cordova-error'),
> >>>>> cdvConfigParser     = require('cordova-config-parser'),
> >>>>> cdvConfigChanges = require('cordova-config-changes');
> >>>>>
> >>>>>
> >>>>> On Thu, Sep 24, 2015 at 12:29 PM Sergey Grebnov (Akvelon) < 
> >>>>> v-segreb@microsoft.com> wrote:
> >>>>>
> >>>>>> Hi guys - we've decided to combine shared logic as 
> >>>>>> cordova-common module and publish it separately (read [1] for
more details).
> >>>>>> Corresponding change has landed to master[2] on last week so

> >>>>>> I'm wondering if we should release this module and then update

> >>>>>> LIB to rely
> >>>>> on it (similar to cordova-serve).
> >>>>>> So guys it will be great if we can review it one more time 
> >>>>>> (including the name - do we all  agree to use cordova-common??)

> >>>>>> and then do release - I'll be able to help w/ merging the 
> >>>>>> recent changes added to LIB before doing release.
> >>>>>>
> >>>>>> [1]
> >>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%
> >>>>>> 2fis
> >>>>>> sue
> >>>>>> s.apache.org%2fjira%2fbrowse%2fCB-9598&data=01%7c01%7cv-segreb%
> >>>>>> 40mi
> >>>>>> cro
> >>>>>> soft.com%7cf31529ebb0de4bf28ebd08d2c54915f3%7c72f988bf86f141af9
> >>>>>> 1ab2
> >>>>>> d7c
> >>>>>> d011db47%7c1&sdata=oeX8CbX%2bQGJsvf9%2fW2KFWAkUw6NAlb0LMOorTjwX
> >>>>>> TXk%
> >>>>>> 3d
> >>>>>> [2]
> >>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%
> >>>>>> 2fgi
> >>>>>> thu
> >>>>>> b.com%2fapache%2fcordova-lib%2ftree%2fmaster%2fcordova-common&d
> >>>>>> ata=
> >>>>>> 01%
> >>>>>> 7c01%7cv-segreb%40microsoft.com%7cf31529ebb0de4bf28ebd08d2c5491
> >>>>>> 5f3%
> >>>>>> 7c7
> >>>>>> 2f988bf86f141af91ab2d7cd011db47%7c1&sdata=o0EwRydALocXUrr4I9ASf
> >>>>>> QMku
> >>>>>> RV0
> >>>>>> ksMKA%2fp2zpg6BNU%3d
> >>>>>>
> >>>>>> Thx!
> >>>>>> Sergey
> >>>>>>
> >>>>>> ---------------------------------------------------------------
> >>>>>> ----
> >>>>>> -- To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>
> >>> ------------------------------------------------------------------
> >>> --
> >>> - To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>> For additional commands, e-mail: dev-help@cordova.apache.org
> >
> > --------------------------------------------------------------------
> > - To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
Mime
View raw message