-
-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Corrected AsyncReaderWrapperForByteBuffer #33
Corrected AsyncReaderWrapperForByteBuffer #33
Conversation
…not AsyncByteArrayFeeder
Nice work Adam. I thought the project was not alive any more. |
@dizzzz Thanks. I am not sure of the status of this project exactly, but I am using it in a tool for obfuscating XML files (which I hope to release as Open Source) and it is working very well for my needs. I chose it because I was particularly interested in using its asynchronous processing. Perhaps @cowtowncoder can give us an idea about any official status of the project? |
@adamretter @dizzzz Project is alive, I just haven't had any need for new features myself (haven't really had need to parse XML myself for couple of years now), so mostly relying on bug reports and contributions like this one. This looks like a nice improvement; let me read it and should be able to merge soon. |
@cowtowncoder Thanks very much. Eventually, I want to expand my tool to also do JSON, perhaps if I am lucky I might find some time to work on FasterXML/jackson-core#57 |
@cowtowncoder great to know it is still alive. Having the project on github really helps already, more or less active maintenance is great! Then it makes sense to actively contribute ..... |
@adamretter In theory building similar JSON codec with a state machine should be smaller undertaking than Aalto. Unfortunately I haven't had enough time or need for non-blocking JSON decoder, so initial plans haven't gone past planning stages. But who knows. @dizzzz Yes, I want to actively at least help merge contributions in, as well as fix bugs that are found. So similar to Woodstox, project is still maintained if not very active. |
One more note: I think upgrade to Java 7 would be ok, esp. for use of diamond patterns. But Java 8 is still bit early as I am bit worried about Android compatibility -- apparently Java 7 language constructors are ok (but not JDK classes necessarily). |
Corrected AsyncReaderWrapperForByteBuffer
@cowtowncoder Okay cool. I am not overly concerned about leaving Java 6 behind, if you want Android compatibility I think you are pretty much stuck there in the medium term. As I understand it Android only supports some features of Java 7 rather than all. |
@adamretter Yes, wrt JDK. For now, I think I'd like to keep code itself Java 6. Perhaps I should go ahead and release official 1.0, and sometime after that increase base level to Java 7. That way we'd have older branch for users on Android (with possibility to patch critical bugs if any) as a fallback. |
If the code base is stable, I'd vote for a 1.0 :) it will provide additional community acceptance anyway... You could keep the 1.x compatible with java6 (sustaining?) and have active development in a 2.x release supporting java7 or even java8 ? |
@cowtowncoder @dizzzz Yes I will need a release (1.0 or 0.7) ideally after #34 |
Previously
AsyncReaderWrapperForByteBuffer
was just a copy ofAsyncReaderWrapperForByteArray
and was not used by any of the tests.AsyncReaderWrapperForByteBuffer
now usesAsyncByteBufferFeeder
as I believe it should.In addition, all the async tests have been updated to test both
ByteBuffer
andbyte[]
code paths.Unfortunately this leads to quite a bit of code duplication in the tests, this could be reduced if we switched to using JUnit annotations and used @RunWith to inject parameters. We could also further reduce code duplication, using
try-with-resources
from Java 7, and function references from Java 8.