Pull requests

#6 Merged
Repository
darjus/jython-proxymaker jython-proxymaker
Branch
default
Repository
python_mirrors/jython jython
Branch
default

Initial implementation of customizable proxymaker with annotation support and reference implementation in tests.

Author
  1. Darjus Loktevic
Reviewers
Description

Please review. It's initial commit so, don't be shy to critique :)

  • Learn about pull requests

Comments (15)

  1. Darjus Loktevic author

    All tests passing, including the new ones.

    javatest:
    [junit] Running com.ziclix.python.sql.DataHandlerTest
    [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 1.057 sec
    [junit] Running org.python.compiler.JavaMakerSmokeTest
    [junit] Tests run: 4, Failures: 0, Errors: 0, Time elapsed: 1.244 sec
    [junit] Running org.python.compiler.ModuleTest
    [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.057 sec
    [junit] Running org.python.compiler.custom_proxymaker.ClassAnnotationTest
    [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 1.224 sec
    [junit] Running org.python.compiler.custom_proxymaker.ConstructorSignatureTest
    [junit] Tests run: 0, Failures: 0, Errors: 0, Time elapsed: 0.046 sec
    [junit] Running org.python.compiler.custom_proxymaker.JUnitTest
    [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 2.218 sec
    [junit] Running org.python.compiler.custom_proxymaker.MethodSignatureTest
    [junit] Tests run: 8, Failures: 0, Errors: 0, Time elapsed: 2.42 sec
    
  2. Frank Wierzbicki

    Finally getting to this - I started reviewing last night and should have it done before tomorrow. Sorry for the slowness - I had to dust off some mental cobwebs for this part of the codebase :)

  3. Frank Wierzbicki

    OK - the code generally looks good to me - but there are some new test failures involving duplicated method names in class files. You can trigger an example of the problem by trying to import subprocess - it causes this:

    java.lang.ClassFormatError: Duplicate method name&signature in class file org/python/proxies/subprocess$_CouplerThread$1

    Maybe your constructor/addMethod combo is causing dup names?

  4. Frank Wierzbicki

    BTW if you run "ant regrtest" with and without this patch applied you can see some of the new failures. Unfortunately we don't have a clean regression test right now - I get about 17 failures normally and 49 with the patch applied (most of these seem to stem from subprocess)

    Again I really appreciate that you are looking at this - the Jython community has been waiting for Clamp for a long long time! And now that I've had a chance to look over the code, reviewing and accepting it should get a lot quicker.