You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Ksenia Khailenko <xe...@tut.by> on 2011/08/20 14:10:53 UTC
gsoc update comments
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
Re: gsoc update comments
Posted by Eshan Sudharaka <es...@gmail.com>.
On Sun, Aug 21, 2011 at 9:52 AM, Eshan Sudharaka <es...@gmail.com>wrote:
>
>
> On Sat, Aug 20, 2011 at 5:40 PM, Ksenia Khailenko <xe...@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.
>
I found a way of implementing
this.example<http://blog.eclipse-tips.com/2008/04/content-type-specific-file-icons.html#comment-form>
Still could not find the default xml editor in eclipse. According to above
we just have to set that default xml editor class in plugins.xml.
>
>> 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/
>
>
--
*~Thanks & Regards~*
***
*
P.A.Eshan Sudharaka
Dept of Computer Science and Engineering
University of Moratuwa
Sri Lanka
http://esudharaka.blogspot.com/
Re: gsoc update comments
Posted by Eshan Sudharaka <es...@gmail.com>.
On Sat, Aug 20, 2011 at 5:40 PM, Ksenia Khailenko <xe...@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/