cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eshan Sudharaka <esudhar...@gmail.com>
Subject Re: gsoc update comments
Date Sun, 21 Aug 2011 04:22:14 GMT
On Sat, Aug 20, 2011 at 5:40 PM, Ksenia Khailenko <xenia_khailenka@tut.by>wrote:

> 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?
>
>
> yes. Just add this to made those lists more configurable.
>
> 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 got those jar files  from eclipse installation directory.



>  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?
>  now i am doing modifications from your patch.
> 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.
>

Here it is working. Can you please clear(delete the content) the manifest
file and
run mvn clean install.

>
> 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.
>
> Reason for these warnings is because it tries to overwrite  the manifest
file. Once we dlete the conten on it and run mvn clean install they will be
no any warnings.

>
> 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.
>
> Now I am working on this issue.

>
> 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.
>

Thanks for the feedback. Will provide a update soon by fixing those.

>
>
> --
> Regards, Ksenia Khailenko
>



-- 
*~Thanks & Regards~*
***
*
P.A.Eshan Sudharaka
Dept of Computer Science and Engineering
University of Moratuwa
Sri Lanka
http://esudharaka.blogspot.com/

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