hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: HC 5.0: HttpCore 5.0 alpha1 release
Date Tue, 01 Dec 2015 20:46:19 GMT
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.

Oleg


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


Mime
View raw message