directmemory-dev mailing list archives

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

Am 30.09.2012 15:05, schrieb Simone Tripodi (JIRA):
> [
> ]
> 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|]
> 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|]
> (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|]
> 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|]
> 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 
>> ---------------------------------
>> 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:

- -- 

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

Meet me at:
Version: GnuPG v2.0.12 (MingW32)
Comment: Using GnuPG with Mozilla -


View raw message