cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ksenia Khailenko <xenia_khaile...@tut.by>
Subject gsoc update comments
Date Sat, 20 Aug 2011 12:10:53 GMT
Hi Eshan! first of all, thanks for the updates.

Here are some comments:

*Cayenne modeler modifications.*

I've committed the needed modifications in cayenne, used your patch from
10.08, "modified.patch". I've changed it a bit, and here are the comments
for your code.

DefaultActionManager:

1.     if(! isActionExist(PROJECT_ACTIONS, actionName)){
         PROJECT_ACTIONS.add(actionName);
        }else{
            //Do nothing if Action is already in the list
        }

if you don't attempt to do anything, the else case with such a comment is
seems to be useless

2.

private boolean isActionExist(Collection<String> actionList, String
actionName) {

        for (String s : actionList) {
            if (s.equalsIgnoreCase(actionName)) {
                return true;
            }
            ;
        }
        return false;
    }


Interface Collection supports method "contains" which works "returns true if
and only if this collection contains at least one element e such that
(o==null ? e==null : o.equals(e))."
As I can see, your method use "equalsIgnoreCase" for comparison, but I don't
see the purpose of that, so, better not to use custom methods for this
operation. And moreover, if you are using the HashSet for these collections,
this check is redundant, but leaving this for now.


3.public void removeProjectaction(String actionName) {
        if (isActionExist(PROJECT_ACTIONS, actionName)) {
            PROCEDURE_ACTIONS.remove(actionName);
        }
        else {
            // Do niothing when the action is not in the list
        }
    }

" PROCEDURE_ACTIONS.remove(actionName); " - I guess, the misprint here?

btw, I don't see if this method is used anywhere… is it?




4.   PROJECT_ACTIONS = Arrays.asList(….

when you are initializing some collection with Arrays.asList, you'll get the
"unmodifiable" list. that's why methods addProjectAction, and
removeProjectAction are not working. I've fixed this(svn commit: r1159871),
but I'm still confused how could it work for you.


*The plugin itself.*


Could you provide the urls which you have used to get libraries from your
repository folder? we will add them to our repository then and I will be
able to commit your results to sandbox.

 I just copied your zipped project and cleaned up some folders("repository"
and "{project.basedir}"). And made the patch from this project. I'll send it
to you, could you apply it to the sandbox and the make the adjustments to
this project for consistency?

Here are some problems with plugin:

1) The opening of modeler works only if I launch plugin as an eclipse
application. If I'm copying the jar to plugins director, it displays the
cayenne icon, but fails to open the modeler.

2)build warnings(from the "mvn clean install" output):
Aug 20, 2011 1:41:10 PM java.util.jar.Attributes read
WARNING: Duplicate name in Manifest: Manifest-Version.
Ensure that the manifest does not have duplicate entries, and
that blank lines separate individual sections in both your
manifest and in the META-INF/MANIFEST.MF entry in the jar file.
Aug 20, 2011 1:41:10 PM java.util.jar.Attributes read
WARNING: Duplicate name in Manifest: Created-By.
Ensure that the manifest does not have duplicate entries, and
that blank lines separate individual sections in both your
manifest and in the META-INF/MANIFEST.MF entry in the jar file.
Aug 20, 2011 1:41:10 PM java.util.jar.Attributes read
WARNING: Duplicate name in Manifest: Built-By.
Ensure that the manifest does not have duplicate entries, and
that blank lines separate individual sections in both your
manifest and in the META-INF/MANIFEST.MF entry in the jar file.
Aug 20, 2011 1:41:10 PM java.util.jar.Attributes read
WARNING: Duplicate name in Manifest: Build-Jdk.
Ensure that the manifest does not have duplicate entries, and
that blank lines separate individual sections in both your
manifest and in the META-INF/MANIFEST.MF entry in the jar file.


2) now only the project file is displayed with the cayenne icon. it was much
better when all the cayenne files was displayed with cayenne icon.


4) go through the classes and clean up unused imports, and the rubbish
comments! old commented code, autogenerated todos, etc. use build in
formatter to format the code.


-- 
Regards, Ksenia Khailenko

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message