[keycloak-dev] Merging node_modules into Keycloak repo

Fri May 12 08:23:14 EDT 2017

On 5/12/2017 5:03 AM, Stian Thorgersen wrote:
> That one looks good. Stan we should give it a go
Sounds like exactly what I've been looking for.  I'll give it a try and 
hope it pans out.

Did someone on this thread mention an internal repo we should be pulling 
from for productization?
>
> On 12 May 2017 at 10:58, Steven Mirabito <stevenmirabito at gmail.com> wrote:
>
>> Looks like this Maven plugin is exactly what you're looking for:
>> https://github.com/eirslett/frontend-maven-plugin
>>
>> As I mentioned before, I would recommend using Yarn over NPM to get the
>> local package caching/offline support that's also desired, which looks like
>> a supported configuration with the aforementioned plugin.
>>
>> -Steven
>>
>> On Fri, May 12, 2017, 1:31 AM Stian Thorgersen <sthorger at redhat.com>
>> wrote:
>>
>>> On 11 May 2017 at 22:06, Bruno Oliveira <bruno at abstractj.org> wrote:
>>>
>>>> Some answers 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.
>>>>
>>> I'm not super keen on that either. I'm not a Node dev and most of the
>>> folks
>>> on the team and our contributors aren't. So this has to be Java folks
>>> friendly.
>>>
>>>
>>>>> 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.
>>>>
>>> What diff are you talking about? RHSSO is just pulling in KC tags. That's
>>> it. However, it might be that we're "cheating" the productization process
>>> by checking in the modules.
>>>
>>>
>>>>>> * 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.
>>>>
>>> Nah you don't. Because Maven has a global Maven repo on your box
>>> (~/.m2/repository dude). As long as you've ran the build since the deps
>>> was
>>> last changed you can just use the offline mode with Maven.
>>>
>>>
>>>>>> * 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?
>>>>
>>> Nah, see above.
>>>
>>>
>>>> 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 :)
>>>>
>>> I'm actually more happy with not checking it in if we can workaround the
>>> issues and make it a Java dev friendly process.
>>>
>>> Can all the stuff needed be retrieved with Maven plugins or would folks
>>> have to install Node, NPM, etc, etc.. to build what is mostly a "Java
>>> project"?
>>>
>>>
>>>>>> 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
>>>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev