On 12 May 2017 at 10:25, Bruno Oliveira <bruno(a)abstractj.org> wrote:
On 2017-05-11, Stan Silvert 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(a)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?
I just asked Node team on it. Will share the answer as soon as I hear
something back.
> >
> > > > * 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?
You have to download once, per project.
Where is it saved? I might be mistake, but I think MVN plugins for this
stuff just puts it in target dir, which is deleted on a mvn clean
>
> 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?
The local repo lives inside the project. But no, there's no concept of
global module for project dependencies and as the author states,
that never gonna happen with npm (
https://github.com/npm/npm/
issues/2949#issuecomment-11408461).
That's just plain out stupid.... Mvn repo in home dir is one of it's
greatest concepts.
> >
> > > > * 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.
If you never downloaded any dependency, I doubt it ;)
As I said you just run mvn build once, then you can use it offline for as
long as you want unless dependencies change.
> >
> > 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(a)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(a)lists.jboss.org
> > > > > > > >
https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > > > > > > --
> > > > > > >
> > > > > > > abstractj
> > > > > --
> > > > >
> > > > > abstractj
> > > > > _______________________________________________
> > > > > keycloak-dev mailing list
> > > > > keycloak-dev(a)lists.jboss.org
> > > > >
https://lists.jboss.org/mailman/listinfo/keycloak-dev
> > > > >
> > > >
> > --
> >
> > abstractj
>
--
abstractj