+1 nice conclusion. On Wed, Oct 28, 2015 at 5:30 PM, Jesse <[email protected]> wrote:
> Yes, I believe the cordova-lib case should be linked, and definitely this > is where a relative link makes sense, since it's all in the same repo. > > > @purplecabbage > risingj.com > > On Wed, Oct 28, 2015 at 5:28 PM, Dmitry Blotsky <[email protected]> > wrote: > > > > trust that each platform will be updated > > > with a newer cordova-common when it makes sense > > > > That sounds reasonable to me. I’m removing the link step for > > cordova-common in platforms. > > > > Last question: the cordova-common used by cordova-lib should still be > > linked, yes? > > > > Kindly, > > Dmitry > > > > > On Oct 28, 2015, at 2:44 PM, Jesse <[email protected]> wrote: > > > > > > It is completely likely, and maybe even expected that the specific > > version > > > of cordova-common that sits in any particular platform template can be > > > different than what is used by cordova-lib. > > > They may even be different amongst platforms in a single project. > > > > > > As long as the lib->platform api is consistent, this is not an issue. > We > > > use cordova-common solely to reduce duplicated code in our repos; We do > > not > > > use cordova-common to reduce the duplicated code on app developers' > > > machines between multiple projects. > > > > > > re: >> Follow-up: the reason we link them is so that we test master > with > > > master at all times. > > > I would change this so that a platform is tested as a complete entity. > > Test > > > what is in platform-master and trust that each platform will be updated > > > with a newer cordova-common when it makes sense. > > > > > > > > > > > > @purplecabbage > > > > > > https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cdblotsky%40microsoft.com%7c25dac595182e43deb77b08d2dfe0ee9c%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=9qhpB1FZ258BqXXE0vEepVP%2fR%2bbxgEViwRhTFEDm6sE%3d > > > > > > On Wed, Oct 28, 2015 at 1:42 PM, Dmitry Blotsky < > [email protected]> > > > wrote: > > > > > >> This use case does mostly affect Cordova contributors. Usually the > > >> platforms will come packaged, you’re right. The discussion of > > >> packaged-vs-installed is separate, but just as a nod to it: I don’t > see > > a > > >> reason we can’t just automatically call “npm install” in > > >> platforms/ios/cordova. Copying over a package.json with fixed versions > > is > > >> no more complex than copying over node_modules. > > >> > > >> Kindly, > > >> Dmitry > > >> > > >>> On Oct 28, 2015, at 1:33 PM, Steven Gill <[email protected]> > > wrote: > > >>> > > >>> Currently, those modules ship with the platform. Example, cordova-ios > > >> will > > >>> have all necessary modules bundled in. When you create a cordova > > project > > >>> and add a platform, it takes those modules and moves them into your > > newly > > >>> created cordova project. To have the cordova project run `npm > install` > > >> and > > >>> install those modules would require us include those modules in a > > project > > >>> level `package.json` file for every cordova project that adds that > > >>> platform. This would confuse developers for sure. I don't think we > > would > > >>> have to modify requires to have this work. > > >>> > > >>> To run `npm install` on those directories would also be very poor > > >> practice. > > >>> By those directories, I assume you mean > > >>> `MYCORDOVAPROJECT/platforms/ios/cordova` (this is where necessary > > >>> node_modules from cordova-ios get copied). It would mean we would > have > > to > > >>> have a `package.json` be copied from `cordova-ios` into > > >>> `MyCordovaProject/platforms/ios/cordova`. A cordova project would > then > > at > > >>> least have as many package.json files as platforms have been added. I > > >> think > > >>> this is poor practice. > > >>> > > >>> Your usecase of npm linking modules in platforms > > >> (cordova-ios/node_modules) > > >>> that then get copied into `MYCORDOVAPROJECT/platforms/ios/cordova` is > > >>> unique. I still think the best solution is to add some sort of check > to > > >>> make sure you haven't npm linked or handle the npm linked case when > > >>> copying. Guess this problem will arise more often with > cordova-common. > > >> That > > >>> usecase pretty much only affects cordova contributors. > > >>> > > >>> -Steve > > >>> > > >>> On Wed, Oct 28, 2015 at 1:18 PM, Dmitry Blotsky < > > [email protected]> > > >>> wrote: > > >>> > > >>>> Is it possible to do “npm install” in those directories instead? Or > to > > >>>> adjust the path so that require() works with the original > node_modules > > >>>> directory? > > >>>> > > >>>> Kindly, > > >>>> Dmitry > > >>>> > > >>>>> On Oct 27, 2015, at 10:31 PM, Steven Gill <[email protected]> > > >>>> wrote: > > >>>>> > > >>>>> I don't think we thought of symlinks in this usecase. Probably > worth > > >>>> adding > > >>>>> in code that checks for symlinks before the copy. I don't see us > > >> removing > > >>>>> this copy as the cordova scripts (build, run, install, etc) require > > >> those > > >>>>> modules. > > >>>>> > > >>>>> On Tue, Oct 27, 2015 at 9:24 PM, Dmitry Blotsky < > > >> [email protected]> > > >>>>> wrote: > > >>>>> > > >>>>>> Ping. Anyone have any information on this? Is it safe to "cp -r” a > > >>>>>> node_modules directory? > > >>>>>> > > >>>>>> Kindly, > > >>>>>> Dmitry > > >>>>>> > > >>>>>>> On Oct 26, 2015, at 3:06 PM, Dmitry Blotsky < > > [email protected]> > > >>>>>> wrote: > > >>>>>>> > > >>>>>>> Hey folks, > > >>>>>>> > > >>>>>>> I’ve come across a bug with symlinks and platform installation > > >>>> recently. > > >>>>>> The point of interest is this line: > > >>>>>> > > >>>> > > >> > > > https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fapache%2fcordova-ios%2fblob%2f4039aeb6f87c6803df5814b8cdefb8c2058504a0%2fbin%2flib%2fcreate.js%23L93.&data=01%7c01%7cdblotsky%40microsoft.com%7c2b31253a23cc4d165ed808d2de51c9ef%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cyXGPBQPr9v46gc6DMDeC9zZkzda8bSCIoinEyAmKrs%3d > > >>>>>>> > > >>>>>>> Is copying node_modules a safe operation? In my case there was a > > >>>>>> relative symlink inside it when it was copied and as a result some > > >>>>>> dependencies broke. The symlink was created by a previous > invocation > > >> of > > >>>>>> “npm link”. > > >>>>>>> > > >>>>>>> Kindly, > > >>>>>>> Dmitry > > >>>>>> > > >>>>>> > > >>>> > > >>>> > > >> > > >> > > > > >
