You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gora.apache.org by lewismc <gi...@git.apache.org> on 2016/08/04 23:02:06 UTC

[GitHub] gora issue #72: Gora 409

Github user lewismc commented on the issue:

    https://github.com/apache/gora/pull/72
  
    @djkevincr NICE WORK :)
    Some comments
     1. Please include Javadoc for all of your publicly available methods, Classes and also add package-info.java to every package you create.
     2. Please ALWAYS catch and log Exception's within your LOG message e.g. ```LOG.error("Log message here", e) 
     3. Please ALWAYS use parameterized logging e.g. ```LOG.info("Log {} and {} values", value1, value2)```
     4. My main question is, what is going on with the testing? Are you setting up the Hazelcast test infrastructure or how is the Mock testing working? This needs to be added to the test class and needs to be explained directly there. Also why are you not extending DataStoreTestBase?
     5. As far as I can see, you've not added this to the gora-tutorial meaning that no-one can run this entire module easily. Please make sure to add it to the gora-tutorial.
     6. Please add this to gora-goraci, we will wish to test it in the Cloud shortly and may even write Docker containers for all of this.
    
    If you can lease address the documentation and logging issues first then notify me that would be great.
    
    Again, great work. Nice to see this coming along.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---