Re: [eclipselink-dev] bug-248780.patch

[Tom Ware <tom.ware@xxxxxxxxxx>]

Who is checking the final changes in?  Mitesh?  Someone else?

Andrei Ilitchev wrote:
> Looks good.
> The only tiny thing - you don't need session if cache already exists 
> (getCache method) - move getSession under if statement.
>
>     ----- Original Message -----
>     *From:* Darani Yallapragada <mailto:Darani.Yallapragada@xxxxxxx>
>     *To:* Dev mailing list for Eclipse Persistence Services
>     <mailto:eclipselink-dev@xxxxxxxxxxx>
>     *Sent:* Wednesday, December 03, 2008 5:45 PM
>     *Subject:* Re: [eclipselink-dev] bug-248780.patch
>
>     Andrei Ilitchev wrote:
> >     There seems to be no need to cache emf inside of CacheImpl -
> >     ServerSession is all that required?
> >     The only usage for emf inside CacheImpl would be to check whether
> >     the factory is still open - and throw exception in each method
> >     otherwise
>     Updated CacheImpl to check whether factory is still open in each method.
> >     Also getCache method should verify whether the factory isOpen -
> >     and fail if it's closed
>       updated getCache() method to verify whether the factory is open.
>
>     Updated patch will be attached to the bug for further review.
>
>     Thank You
>
>     Regards
>     Darani
>
> >         ----- Original Message -----
> >         *From:* Gordon Yorke <mailto:gordon.yorke@xxxxxxxxxx>
> >         *To:* Dev mailing list for Eclipse Persistence Services
> >         <mailto:eclipselink-dev@xxxxxxxxxxx>
> >         *Sent:* Wednesday, December 03, 2008 11:34 AM
> >         *Subject:* Re: [eclipselink-dev] bug-248780.patch
> >
> >         CacheImpl.java - createPKVector() -> no need to create local
> >         variables, just use method parameters.  Defining local
> >         variables could introduce errors and looks cluttered.
> >         EntityManagerFactoryImpl.java -> make reference to cacheImpl
> >         'protected'.  This is an EclipseLink coding pattern.
> >
> >         Otherwise this looks great.
> >         --Gordon
> >
> >         Darani Yallapragada wrote:
> > >         Hello :
> > >
> > >         Fix for Bug#248780 is attached as "bug-248780.patch" file.
> > >         Please do review this file .
> > >
> > >         Thank You
> > >
> > >         Regards
> > >         Darani
> > >
> > >
> > >
> > >         ------------------------------------------------------------------------
> > >
> > >         _______________________________________________
> > >         eclipselink-dev mailing list
> > >         eclipselink-dev@xxxxxxxxxxx
> > >         https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
> >
> >         ------------------------------------------------------------------------
> >         _______________________________________________
> >         eclipselink-dev mailing list
> >         eclipselink-dev@xxxxxxxxxxx
> >         https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
> >
> >     ------------------------------------------------------------------------
> >
> >     _______________________________________________
> >     eclipselink-dev mailing list
> >     eclipselink-dev@xxxxxxxxxxx
> >     https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
> >       
>
>     ------------------------------------------------------------------------
>
>     _______________________________________________
>     eclipselink-dev mailing list
>     eclipselink-dev@xxxxxxxxxxx
>     https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev