[keycloak-dev] Merging node_modules into Keycloak repo

Fri May 12 03:35:09 EDT 2017

Is that yet another thing I've got to install ;)

As a Java dev I just wanna install the JDK and Maven then run "mvn clean
install". I don't want to have to install a bunch of Node stuff manually.

On 12 May 2017 at 03:25, Steven Mirabito <stevenmirabito at gmail.com> wrote:

> Using yarn instead of npm to manage node_modules (it's a drop-in
> replacement in most cases) will provide a local package cache similar to
> the local Maven repo: https://yarnpkg.com
>
> -Steven
>
> On Thu, May 11, 2017, 5:57 PM Stan Silvert <ssilvert at redhat.com> wrote:
>
>> On 5/11/2017 4:06 PM, Bruno Oliveira wrote:
>> > Some answers inline.
>> More comments inline...
>> >
>> > On 2017-05-11, Stian Thorgersen wrote:
>> >> Oh and also I like to just run stuff from the IDE. I very rarely build
>> >> Keycloak from mvn unless I'm mocking about with the distribution parts.
>> > To run stuff from your IDE, you just run npm install inside the project.
>> > You don't need Maven for it.
>> >
>> >> On 11 May 2017 at 19:17, Stian Thorgersen <sthorger at redhat.com> wrote:
>> >>
>> >>> I have 3 concerns around not checking in node_modules. If these are
>> >>> addressed I'm happy:
>> >>>
>> >>> * Productization - I believe not checking in the modules will mean
>> they
>> >>> have to be available in some internal NPM repository or something.
>> Those
>> >>> module not already available we'll have to add ourselves. Stan you
>> would
>> >>> have to figure this out as we no longer have a dedicated prod team
>> (that's
>> >>> us now!)
>> > Yep, I know. One of the reasons why run diff at each upgrade of these
>> > dependencies will sound like hell.
>> I don't know the answer to the productization question.  SOMEBODY in
>> MiddleWare
>> must have dealt with this before.  Who can I ask?
>> >
>> >>> * Build time - just the basic Angular2 example adds overhead
>> > I won't take the road of Java vs Node.js. But pretty much you have the
>> > same issue with Maven, when you don't have dependencies installed.
>> > One way or another you have to download it from the internet.
>> With Maven, you only have to download once.  But with node_modules not
>> checked in
>> you will have to download every time after you do a clean.  Or is there
>> some way around that?
>>
>> npm doesn't seem to have the same concept of a local repo like Maven.
>> Installing globally doesn't fix
>> the problem.  Or is there a solution I don't know about?
>> >
>> >>> * Offline mode - the Angular2 example at the moment can't be built
>> when
>> >>> not online. It will sit around for a loooong time until it eventually
>> times
>> >>> out. NPM modules would have to be checked in somewhere globally (not
>> just
>> >>> inside target dir or something like that). Maybe they can just be
>> placed
>> >>> inside the .m2 dir or something?!
>> > Isn't the same when you are disconnected and you try to run Maven?
>> No.   You can rely on the local Maven repo.  Works great disconnected.
>> >
>> > Anyways, I don't think we gonna reach any consensus about it. So,
>> anything
>> > you guys think is the best for the project, go ahead. It's software, we
>> > can always change, sometimes break :)
>> >
>> >>> On 11 May 2017 at 15:10, Bruno Oliveira <bruno at abstractj.org> wrote:
>> >>>
>> >>>> On 2017-05-11, Stan Silvert wrote:
>> >>>>> On 5/10/2017 6:14 PM, Bruno Oliveira wrote:
>> >>>>>> On 2017-05-10, Stan Silvert wrote:
>> >>>>>>> The very thought of $subject seems like heresy.  Why check in
>> >>>> something
>> >>>>>>> that is normally pulled using npm?
>> >>>>>>>
>> >>>>>>> We have Angular 2 examples in Keycloak now.  In the
>> not-too-distant
>> >>>>>>> future, our Account Management console will be written in Angular
>> >>>> 2.  So
>> >>>>>>> node_modules has to be there somehow.
>> >>>>>>>
>> >>>>>>> There are basically two options:
>> >>>>>>> 1)  Merge node_modules into the Keycloak repo.
>> >>>>>>> 2)  Don't merge and then run npm install at build time.
>> >>>>>>>
>> >>>>>>> Productization standards push toward option #1.  We need to have
>> >>>>>>> consistent, repeatable builds.
>> >>>>>>>
>> >>>>>>> But I'm looking for reasons that #1 might be bad.  I can't come up
>> >>>> with
>> >>>>>>> a rational reason to do #2 except that it saves disk space.
>> >>>>>> My 2 cents here. I think #1 is bad for the following reasons:
>> >>>>> My instinct is that it's bad too.  But I need to play devil's
>> advocate.
>> >>>>>> 1. That's not the convention for Node.js development. Think about
>> Java
>> >>>>>> devs committing JARs to our repo. Certainly that would be terrible.
>> >>>>> That's just the heresy argument.
>> >>>> Not really. It's also the way which QE thinks is the way to go (
>> >>>> https://github.com/keycloak/keycloak-quickstarts/pull/25#is
>> >>>> suecomment-300751341)
>> >>>>
>> >>>>>> 2. Code review becomes a PITA at every dependency update.
>> >>>>> If you are talking about review in GitHub, that shouldn't be a
>> problem.
>> >>>>> GitHub uses linguist to hide vendor files so that it doesn't mess up
>> >>>> your
>> >>>>> review.  All you see is the file name.  By default, it doesn't show
>> you
>> >>>> the
>> >>>>> whole file.  Strangely enough, the linguist guys see checking in js
>> >>>>> libraries as a common practice:
>> >>>>> https://github.com/github/linguist#vendored-code
>> >>>> Well, you're assuming that we should all do our code reviews using
>> only
>> >>>> GH.
>> >>>>
>> >>>>>> 3. Merge conflicts. Just think about multiple devs contribution to
>> the
>> >>>>>> same repo and updating their modules.
>> >>>>> Maybe I'm missing something here.  Wouldn't that be really rare if
>> it
>> >>>> ever
>> >>>>> happened at all (in our case)?
>> >>>> Not if every developer decide to run npm update :)
>> >>>>
>> >>>>>> 4. People could manually change these modules and you would never
>> know
>> >>>>>> if the change is a consequence of `npm update` or a manual change.
>> >>>>> I really hope nobody would be dumb enough to do that!
>> >>>>>
>> >>>>> But come to think of it, this would be a security issue if someone
>> did
>> >>>> it on
>> >>>>> purpose.   Someone could sneak malicious code into our repo
>> disguised
>> >>>> as a
>> >>>>> legit PR.  It wouldn't be easy to catch during code review.
>> >>>> Trust me, if people wanted to do that. They wouldn't be dumb just
>> >>>> a single line
>> >>>>
>> >>>>>> 5. This only makes sense on scenarios where dev cannot rely on npm
>> >>>>>> dependencies.
>> >>>>>>
>> >>>>>>
>> >>>>>> IMO option #2 would be the most viable. Projects from RedHat
>> already
>> >>>> do
>> >>>>>> this[1] with some success. So I don't see any need for it.
>> >>>>>>
>> >>>>>> [1] - https://github.com/aerogear/aerogear-unifiedpush-server/blob
>> >>>> /02b133ffb49677effa347788c28c392ed3f275f0/admin-ui/pom.xml#L42
>> >>>>> Doesn't this make the build a LOT slower?
>> >>>> If slowness is the major concern, we should commit every single JAR
>> >>>> inside the repo and call it a day. But I don't think we do that,
>> right?
>> >>>> Plus, Travis can cache the dependencies[1].
>> >>>>
>> >>>> [1] - https://docs.travis-ci.com/user/caching/
>> >>>>
>> >>>>> How is Aerogear dealing with productization?
>> >>>> You have to ask them, more precisely Matthias.
>> >>>>
>> >>>>>>
>> >>>>>>> Any thoughts?
>> >>>>>>> _______________________________________________
>> >>>>>>> keycloak-dev mailing list
>> >>>>>>> keycloak-dev at lists.jboss.org
>> >>>>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >>>>>> --
>> >>>>>>
>> >>>>>> abstractj
>> >>>> --
>> >>>>
>> >>>> abstractj
>> >>>> _______________________________________________
>> >>>> keycloak-dev mailing list
>> >>>> keycloak-dev at lists.jboss.org
>> >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >>>>
>> >>>
>> > --
>> >
>> > abstractj
>>
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>