[keycloak-dev] Merging node_modules into Keycloak repo

Thu May 11 18:22:28 EDT 2017

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