mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "haosdent huang" <haosd...@gmail.com>
Subject Re: Review Request 33339: Add a Java example framework to test persistent volumes.
Date Sun, 21 Jun 2015 05:19:32 GMT


> On June 1, 2015, 11:32 p.m., Marco Massenzio wrote:
> > This is great -sorry it took so long to get to do a review.
> > 
> > Thanks for doing it, I'm quite looking forward to using it to learning more about
the Persistent Framework :)
> > it would be great if we could have a bit more comments in the code to help all other
newbies.
> > 
> > My review was fairly narrowly focused on Java style etc. - I'm expecting one of
the commiters will be able to give you feedback about using the framework.
> 
> Jie Yu wrote:
>     Sorry, this is my fault. I really don't have cycle for this at this moment. Marco,
mind sherperding this patch?
> 
> Jie Yu wrote:
>     The logic of this patch should match that in src/examples/persistent_volume_framework.cpp
> 
> Marco Massenzio wrote:
>     Jie - I would be absolutely happy to do that, with one minor hitch: I'm not a committer
:)
>     Happy to help out wherever I can, though!
> 
> haosdent huang wrote:
>     Thank you very much for your review, let me update it. @marco
> 
> haosdent huang wrote:
>     @marco Thank you for your review again. But some logic are follow the exists Java
examples and persistent_volume_framework.cpp . I am not sure change it are correct or not.
Anyway, I think I need to reflect this patch to make it more clear. After I reflect it, I
would @you and looking forward your review again. Thanks a lot.
> 
> Marco Massenzio wrote:
>     I understand that - and therein lies the problem :)
>     As you proved, code in 'examples' is widely "used as a template" (aka, copied) so
it would be great if we could set an... example, by providing great code there.
>     
>     Translating Java almost verbatim from C++ actually results in non-idiomatic constructs
that either (a) look weird to Java practitioners or (b) engender bad practices (that are then
propagated by the "oh, but that's how it's done in the other examples" - sounds familiar?
:)
>     
>     My suggestions (and I stress it here, they are "suggestions" - if committers with
more Java experience than I are happy with your code, then that's just fine) are to provide
some great examples, that improve on existing code.
>     
>     Thanks for doing this!
> 
> Adam B wrote:
>     Thanks haosdent for coding this up, and thanks Marco for the initial review. As a
Java/C++ polyglot, I'll volunteer to Shepherd this patch, if Jie will help verify that it
properly tests his feature.
>     Unfortunately, it seems the authors of the original Java examples/tests were not
hardened Java developers, and their poor style habits have been copied into subsequent examples.
I won't ask haosdent to solve all of those issues, or else this patch would be indefinitely
delayed. As long as it's no worse than the other examples, and adequately demonstrates/tests
the feature, I'll let it through.

@marco, @adam-mesos I reflect the code, could you review it again? Thank you very much.


- haosdent


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33339/#review86113
-----------------------------------------------------------


On June 21, 2015, 5:17 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33339/
> -----------------------------------------------------------
> 
> (Updated June 21, 2015, 5:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Marco Massenzio.
> 
> 
> Bugs: MESOS-2610
>     https://issues.apache.org/jira/browse/MESOS-2610
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add a Java example framework to test persistent volumes.
> 
> 
> Diffs
> -----
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/examples/java/TestPersistentVolumeFramework.java PRE-CREATION 
>   src/examples/java/test-persistent-volume-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp 2ff6e7a449cc5037f9a3c8d6938855c35e389cca 
>   src/tests/java_persistent_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33339/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


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