johnzon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: R: PULL 17
Date Tue, 27 Mar 2018 10:49:30 GMT
Hi Alessandro, comment on github directly. but high level except the "type"
issue I mentionned and the formatting it looks good.


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>

2018-03-27 12:04 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:

> Ok I just made a new pull request (#18) containing  only support for
> injectable JsonbConfig via JAX-RS Provider.
> I didn’t use Priorities (so it will work on older provider), I didn’t
> create a new library (so it will work out of the box) but I used Priority
> to set a value lower than User default one (4900, so that provider created
> by user will be used over this by default).
> I used geronimo dependencies as you said for point 1.
> I fixed what you said regarding point 4 :
> Now if context resolver is found, the type passed is always propagated.
> If context resolver is not found, type passed is not used, since it was
> not used before my change.
>
> I’ll commit next set of changes as soon as you accept this pull request.
>
> Please let me know as soon as possible.
>
> Thank you
> AM
>
> Da: Romain Manni-Bucau<mailto:rmannibucau@gmail.com>
> Inviato: martedì 27 marzo 2018 11:44
> A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
> Oggetto: Re: R: PULL 17
>
> We can do as for our "mapper" jaxrs provider and support another
> constructor.
> For the priority I wonder why the default/implicit priority doesn't work?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibuca
> u> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-
> high-performance>
>
> 2018-03-27 11:09 GMT+02:00 Alessandro Moscatelli <
> alessandro.moscatelli@live.com>:
>
> > I use Wildfly 12.
> > I am going to make a small pull request with Injectable JsonBconfig a few
> > minutes from now …
> >
> > Greetings
> > AM
> >
> >
> >
> > ________________________________
> > Da: Romain Manni-Bucau <rmannibucau@gmail.com>
> > Inviato: Monday, March 26, 2018 11:21:20 PM
> > A: dev@johnzon.apache.org
> > Oggetto: Re: R: PULL 17
> >
> > Le 26 mars 2018 23:13, "Alessandro Moscatelli" <
> > alessandro.moscatelli@live.com> a écrit :
> >
> > I am not sure I got all you said but let’s divide et impera :
> >
> > 1) I really tried (I have/had no idea what geronimo dep were but I
> figured
> > out javaee dependencies were inside them) but at first I didn’t find the
> > geronimo dep containing Priority annotation. Now I got it :
> > https://mvnrepository.com/artifact/org.apache.geronimo.
> > specs/geronimo-annotation_1.3_spec/1.0
> >
> >
> > Exactly
> >
> >
> >
> > 2) I simply didn’t want to reinvent the wheel, but … oh well I am going
> to
> > find another implementation able to recursively explore superclasses and
> > superinterfaces to find the generics type … are maven dependencies ok ?
> >
> >
> > We really try to limit dependencies at maximum. Shading can be an option
> is
> > size is very small (commons has thus kind of code IIRC) or just do it
> from
> > scratch maybe, it is not that long i think.
> >
> >
> > 3) I think the best is creating an external library. Jsonb is not really
> > related to jax-rs, so maybe a jsonb-jaxrs library is the best option. Is
> > this ok for you ?
> >
> >
> > Was in jsonb cause 1. We sere not java 8 in jaxrs module and 2. It must
> > work ootb in a jaxrs container. That said we can probably disable it
> > somehow. Why container do you use?
> >
> >
> > 4) Here you lost me … completely … what ? “^^
> >
> >
> >  You pass type in the resolver but dont respect this at runtime (let say
> > first call uses Book and init jsonb and next one has User)
> >
> >
> >
> > 5) Oh … ok I’ll try to make some test for the new supported scenarios …
> >
> > 6) I’ll cancel the current pull Tomorrow and try with smaller individual
> > ones.
> >
> >
> >
> > Thanks a LOT for 5 and 6. I appreciate it a lot.
> >
> >
> > Thanks
> > AM
> >
> > Da: Romain Manni-Bucau<mailto:rmannibucau@gmail.com>
> > Inviato: lunedì 26 marzo 2018 20:43
> > A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
> > Oggetto: Re: PULL 17
> >
> > Hi
> >
> > Alessandro, some general comments:
> >
> > 1. Please use geronimo spec dependencies (instead of javax) since asf
> owns
> > it (less legal work + we can tune them if needed)
> > 2. Dont use google deps (we dont want any dep for container control on
> > dependencies)
> > 3. Priority should work on javaee 6 containers so maybe make it optional
> or
> > dont use priorities (annotation are ignored if missing but not classes
> > iirc)
> > 4. Your jsonb type key doesnt work since the instance is for all matching
> > type no?
> > 5. I dont see much tests ;)
> > 6. Surely split in as much pr as change to ease the merge and review
> >
> > Hope it helps, impatient to get it in and congrats for your PR :)
> >
> >
> > Le 26 mars 2018 20:08, "Alessandro Moscatelli" <
> > alessandro.moscatelli@live.com> a écrit :
> >
> > > Hi everybody !
> > > I want to contribute and I made some minor fixes and I am trying to
> pull
> > > my changes with github.
> > >
> > > This is my changelist :
> > >
> > > JAX-RS MessageWriter/MessagerReader with Priority (so that user can
> > define
> > > and provide his own)
> > > JsonbConfig injectable via Jax-RS API
> > > Better support for generics types
> > > Support for JsonbDeserializers/JsonbSerializers defined in interfaces
> or
> > > abstract classes
> > > Support for default deserialization from string to enum
> > > Fix to dateformatting (date format was not properly used in
> > > deserialization)
> > >
> > > I hope you will appreciate …
> > >
> > > Have a nice evening !
> > >
> > > Lemme know what you think, I’d really love to drop my snapshot
> dependency
> > > soon 😊
> > >
> > > Alessandro Moscatelli
> > >
> >
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message