[keycloak-dev] Merging node_modules into Keycloak repo

Stian Thorgersen sthorger at redhat.com
Fri May 12 05:03:04 EDT 2017 

That one looks good. Stan we should give it a go

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
>>
>

More information about the keycloak-dev mailing list