On 11 May 2017 at 22:06, Bruno Oliveira <bruno(a)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(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.
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(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