[vmkit-commits] [PATCH] Various patches

Nicolas Geoffray nicolas.geoffray at gmail.com
Wed Oct 19 02:16:00 PDT 2011


Hi Will,

On Tue, Oct 18, 2011 at 7:56 PM, Will Dietz <willdtz at gmail.com> wrote:

> On Tue, Oct 18, 2011 at 11:39 AM, Nicolas Geoffray
> <nicolas.geoffray at gmail.com> wrote:
> > Hi Will,
> > That's great! Here's my first comments. I'll try to review your other
> > patches ASAP. Do you have commit access to the llvm repo?
>
> It appears I do have commit access.  Not knowing a better way to
> verify if I did, I just tried committing the first three patches that
> you approved and it seems that it went through fine.  While the
> patches were approved and fairly small, hope you don't mind that I
> went ahead and committed them.
>

No problem.


>
> On a related note, humorously enough I have commit access but the
> commits mailing list is grumpy :).  Should I issue some kind of
> subscribe request?  I'm guessing it's because I'm on the list as
> willdtz at gmail, and commit as wdietz2 at uiuc.edu...
>

Coud you try to be on the list as @uiuc? It'd be annoying not to see your
changes in the ML.


>
> > patches 1, 2, 3: look good!
> > patch 4: looking at the code, I realize that NATIVE_JNI is completely
> bogus
> > today, so you can safely remove the use of the macro and the arguments it
> > surrounds. Also, even though it's just casts, you should not use jobject
> and
> > jclass in these methods. Just use JavaObject. Also, please remove the //
> > printf.
>
> Okay, sure thing.  If NATIVE_JNI is bogus, I'll put together a patch
> ripping it out at some point, if you don't beat me to it :).
>

That'd be nice. Thanks!

Nicolas


>
> Sorry about the debugging printf, whoops.
>
> > patch 5: looks good!
>
> Great! :)
>
> Thanks for the quick response and your comments!
>
> ~Will
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111019/ae1e74a3/attachment.html>


More information about the vmkit-commits mailing list