hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Osipov <micha...@apache.org>
Subject Re: HC 5.0: HttpCore 5.0 alpha1 release
Date Tue, 01 Dec 2015 20:50:29 GMT
Am 2015-12-01 um 21:46 schrieb Oleg Kalnichevski:
> On Tue, 2015-12-01 at 21:29 +0100, Michael Osipov wrote:
>> Am 2015-12-01 um 14:35 schrieb Oleg Kalnichevski:
>>> Folks
>>>
>>> I would like to start cutting RC1 for HttpCore 5.0 alpha1 sometime soon.
>>>
>>> Could you please take a look at the latest snapshot and let me know if
>>> you find anything that might block the release or cause another RC
>>> build?
>>
>> While this is an alpha release, the code does not show overall good
>> fitness. It will take a look of tweaks for GA.
>>
>> I've made a quick code review and here are my findings:
>>
>> 1. What about the artifact ids? Didn't we want to apply the same scheme
>> as Apache Commons?
>>      httpcore5-parent
>>      |- httpcore5
>>      |- httpcore5-ab
>>      etc..
>>
>> 2. httpcore/src/main/appended-resources/META-INF/...
>>      This can be autogenerated with
>> https://maven.apache.org/apache-resource-bundles/
>>
>> 3. version.properties requires a relocation and an update
>>
>> 4. Remove all not used Subversion properties like Id, Author, etc. but
>> not eol-style of course.
>>
>> 5. How far are the imported annotations used throughout the code? Are
>> the annotated classed analyzed at runtime or do they serve mere
>> documentation which Javadoc could do?
>>
>> 6. Rename "io" packages to "bio" to make distinction from "nio" (uniform
>> naming)
>>
>> 7. Several comments still refer to the old RFC. Javadoc need to be
>> proof-read and updated to the new RFCs.
>>
>> 8. HttpResponse throws IllegalStageException for illegal arguments. Weird?!
>>
>> 9. Buffer sizes vary from 2 KiB to 8 KiB throughout the code. What about
>> normalization?
>>
>> 10. ContentType: character encoding of application/*-xml is set to
>> ISO-8859-1 but the default encoding for XML is UTF-8.
>> (I am ignoring the XML prolog for now)
>>
>> 11. HeaderGroup: Using a get(int), set(int, Object) on a list (here
>> List<Header>) might incur a performance penalty because they were not
>> designed for that.
>>
>> 12. TokenParser duplicates final statics from Chars
>>
>> 13. Several methods throughout the code declare throws
>> DerivedFromRuntimeException" this is code smell and needs to be removed.
>> Unchecked exception appear only in Javadocs.
>>
>> 14. HttpDateGenerator is marked as @ThreadSafe but SimpleDateFormat is
>> not. WTF?
>>
>> 15. Several wellknown literals are used where statics would be
>> appropriate like 80, 443, HTTP methods, "http", "https" etc.
>>
>> 16. System.currentTimeMillis() is used at several spots where elapsed
>> time is measured. This approach is obsolete/error-prone and shall
>> replaced with System.nanoTime()
>>
>> 17. ByteBufferAllocator and its implementors seem to be pretty much
>> useless/overdesigned just to wrap one method: allocate/allocateDirect,
>> aren't they?
>>
>> 18. Replace Args, Asserts, TextUtils, LangUtils, SimpleDateFormat (#14)
>> with Commons Lang. There is so much duplicate code that it is worth
>> having Commons Lang 3 as dependency.
>>
>> 19. What I absolutely do not like/understand is the inconsistent
>> approach of custom exceptions (not a flame war here please):
>>       * At several points default exceptions are used inconsistently or
>> do not even make sense
>>       * There is no clear distinction between recoverable and
>> unrecoverable exceptions, e.g.,
>>         * why do I have catch a ParseException where I cannot do anything
>> about it?
>>         * I cannot retry anyway when there is malformed input.
>>    * Server sends an HTTP message not conforming with the RFCs, I cannot
>> do anything about it in my code either.
>>
>>      The exceptions require a major overhaul in this new major version.
>>
>> I would work on them piece by piece if no one objects. If I need some
>> guidance, I will ask separately on the list. Feel free to comment on
>> those but please delete the unnecessary parts of this mail.
>>
>
> I _very_ much object to 13, 14, 16 to 19, _especially_ 18 and 19. I am
> not in favor of point 6.

Can you please elaborate on your objection?

Michael


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message