-----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: [email protected] -----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-----
