directmemory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Noctarius ...@noctarius.com>
Subject Re: [jira] [Commented] (DIRECTMEMORY-102) Lightning Serializer Contribution
Date Sun, 30 Sep 2012 14:16:18 GMT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 30.09.2012 15:05, schrieb Simone Tripodi (JIRA):
> 
> [
> https://issues.apache.org/jira/browse/DIRECTMEMORY-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13466467#comment-13466467
> ]
> 
> Simone Tripodi commented on DIRECTMEMORY-102: 
> ---------------------------------------------
> 
> Hi Christoph,
> 
> thanks for contributing! Patch looks quiet good, I'd followup
> the discussion on the dev@ ML first, I have some observations
> about the inclusion before applying it. I am going to send a
> message following up the current thread in a short while. 
> Thanks *a lot* for the hard work and congrats for that lib!
> 
> In the case you are looking for some feedbacks, follow below
> few (ASF general) suggestions to submit patches:
> 
> _Higher priority_
> 
> * serializer modules are put under the
> [serializers|https://svn.apache.org/repos/asf/directmemory/trunk/serializers/]
> directory;

Like Raffaele mentioned the main serializer should be a subproject
and only the DirectMemory connector will be in serializers
subdirectory. That was what we discussed yesterday.

> 
> * no tabs; 2 spaces for XML sources, 4 spaces for Java sources
> - generally, please respect the original source code format,
> people here if following the Apache Maven [code
> conventions|http://maven.apache.org/developers/conventions/code.html]
> (IDEs config included);

That would be no problem, I forgot to ask about a conventions
configuration :)

> 
> * According to other serializer modules, the package should be
> moved to {{org.apache.directmemory.serialization.lightning}};

See above.

> 
> * No needs to define and implement yet another logging
> abstraction level/fa├žade:
> 

It is just used to not depend on one single logging solution. It's
a pain to integrate logging frameworks when you need to integrate
different packages into one big framework. The only solution in
such a case is to use slf4j to redirect everything (log4j, juli, ...).

> * No needs to define a {{Marshaller}}/{{Unmarshaller}}, the
> reference interface can be directly the
> [org.apache.directmemory.serialization.Serializer|https://svn.apache.org/repos/asf/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/serialization/Serializer.java]
> class;

Since Lightning will be usable without DirectMemory there is a
need to do so.

> 
> _Lower priority_
> 
> * please name patch file with the issue key, i.e.
> {{DIRECTMEMORY-102.patch}}, it helps committers that are
> reviewing and applying patches;

Ok noticed.

> 
> * In the following code:
> 
> {code} +@SuppressWarnings("serial") +public class
> ClassDefinitionInconsistentException extends RuntimeException
> { {code}
> 
> you don't need to suppress the serial, you can add
> serialVersionUID field;
> 

Normally I don't like to define serialVersionUIDs because they are
never updated when the class changes and then they "break" the
serialVersionUID contract but no problem I even can add them :-)

> * same for {{IllegalAccessorException}};
> 
> * same for {{IllegalPropertyAccessException}};
> 
> * same for {{SerializerDefinitionException}};
> 
> * same for {{SerializerExecutionException}};
> 
> * same for {{SerializerMarshallerGeneratorException}};

See above.

> 
> * Please drop {{@author}} tags, feel free to add yourself in
> the {{contributors}} section in the parent POM - this is the
> right place where people are enlisted;
> 

It can't be dropped for all classes (I guess) since there are a
few classes I'm not the original author off, so that needs to be
mentioned somewhere. Is there a better / typical place for things
like this in the Apache guidelines?

> * no needs to define a {{StringUtil}} class unless
> [Guava|http://code.google.com/p/guava-libraries/wiki/StringsExplained]
> doesn't provide the functionalities you need; the DirectMemory
> core module relies on Guava;

I'm not sure if I want to depend on guava just for a String
utilities class or at least 2 or 3 methods of them.

> 
> * same for {{TypeUtil}}

See above.

> 
> * why {{Benchmark}} class is annotated with {{@Ignore}}?

Because I don't want to execute it every time, it's just what it
is called a benchmark which I use from time to time to see and
profile performance.

> 
>> Lightning Serializer Contribution 
>> ---------------------------------
>> 
>> Key: DIRECTMEMORY-102 URL:
>> https://issues.apache.org/jira/browse/DIRECTMEMORY-102 
>> Project: Apache DirectMemory Issue Type: New Feature 
>> Components: Serializers Reporter: Christoph Engelbert 
>> Attachments: lightning_contribution.patch
>> 
>> 
>> This is the first contribution patch attempt for the
>> lightning serializer. If there are any things need to be
>> changed please let me know. PS: The issue tracker and
>> sourcelocation values in the pom.xml aren't set yet, since I
>> had no clue what are the correct values but I guess that
>> could be set later on.
> 
> -- This message is automatically generated by JIRA. If you
> think it was sent incorrectly, please contact your JIRA
> administrators For more information on JIRA, see:
> http://www.atlassian.com/software/jira
> 


- -- 


##############################
# A Digital's Life           #
##############################
Nickname: Noctarius
Location: Germany

Meet me at:
Ohloh: http://www.ohloh.net/accounts/noctarius
Web: http://www.noctarius.com
XMPP/Jabber: noctarius@jabber.ccc.de
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQaFQyAAoJEH/g+YBfahrqGv0P/RAIWYxvm4OLqDivhinnq4pP
GIttzy3SA4p/hUuUHbzUtX/VN4kdXHHEZumS3U01YTKGG11WCaDiKlUqMEA47Q2g
bMaTAUe3bfKB3d7o87jH+ZRvTn+/1F8IaGuz3k+V850bm/mzaiZSt7ARLHKo4+Fv
kiakVMLGyQGvFCZPwbYlc2tWdyttu9/Og4OvOOiFBplyQmyNCeEHWs06FCjDzqrv
SRwM5taJdGiEXbgruzY0PEclhqBVPNLZsK1s+tCRFJRq5ENike6cpQIZgf2WgJQ1
nNzfC4taG+VhGihIqznD9SkRiRBRrTTK4MTnaLpQ1EVE3W+P5tf77t6y0F7h04D9
0Kt55GzWkDjPGgcQZe5sDgCvc9VtQpexFw8SwtA+1trc4Y+4wm56C+gRKaruSthT
HHQHkMQmEH1zwBY6zseAHcDh7O2HDDvyZUQN/NUFe2lJMRgnfxmWZoqNrGTgqSDm
SW4o6gKGw+XlZM8XifZsZxh59Ce8X+cQqhtTJrjRQW7cPreQ7O4x7lN3OVOumt8X
NYUsjICCScGpccY/QuqPtXatXXiwPrObi2pQ7YHRMfZx6TqpjW4rxACsu9CUYkS4
knclF9zHK6xoOH1s+B8ZaUMRwGiGk6NN5/lPWiYr/CNxQwexX4AT9taphbuB2q/A
FJNttpE7o/zepgElFtEj
=9GFJ
-----END PGP SIGNATURE-----

Mime
View raw message