On Tue, 2015-12-01 at 21:50 +0100, Michael Osipov wrote: > 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? >
Of course, I can. Some points, like point 14, are just plain wrong. Others are highly disputable like 18 and 19. Let's concentrate on things that objectively need fixing. Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
