From: SourceForge.net <no...@so...> - 2011-06-20 23:12:09
|
Patches item #3323501, was opened at 2011-06-21 00:12 Message generated for change (Tracker Item Submitted) made by asadrahman You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-06-21 08:27:46
|
Patches item #3323501, was opened at 2011-06-21 01:12 Message generated for change (Comment added) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- >Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-06-21 08:34:25
|
Patches item #3323501, was opened at 2011-06-21 00:12 Message generated for change (Comment added) made by asadrahman You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 09:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 09:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-06-21 08:42:05
|
Patches item #3323501, was opened at 2011-06-21 01:12 Message generated for change (Comment added) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- >Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 10:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-07-26 11:23:18
|
Patches item #3323501, was opened at 2011-06-21 00:12 Message generated for change (Comment added) made by asadrahman You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-07-26 12:23 Message: Status update of the patches after running the QA. ant -Dmodule=smsd qa-module Buildfile: /users/Asad/Software/GITROOT/cdksmsdgithub/build.xml checkPlatforms: check: dist.init: qa-module: [echo] Running QA for CDK's smsd module checkPlatforms: check: noJunit: test-module: [echo] Testing classes for CDK's smsd module. [echo] CDK dependencies defined: true [echo] Library dependencies defined: true [echo] Developer Library dependencies defined: true [junit] Running org.openscience.cdk.modulesuites.MsmsdTests [junit] Tests run: 222, Failures: 2, Errors: 0, Time elapsed: 8.498 sec [junit] Test org.openscience.cdk.modulesuites.MsmsdTests FAILED ojdcheck-module: [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/ojdcheck [javadoc] Generating Javadoc [javadoc] Javadoc execution [javadoc] Loading source files for package org.openscience.cdk.normalize... [javadoc] Loading source files for package org.openscience.cdk.smsd... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.matchers... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcgregor... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcsplus... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.rgraph... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.single... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.builder... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.map... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.query... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.substructure... [javadoc] Loading source files for package org.openscience.cdk.smsd.filters... [javadoc] Loading source files for package org.openscience.cdk.smsd.global... [javadoc] Loading source files for package org.openscience.cdk.smsd.helper... [javadoc] Loading source files for package org.openscience.cdk.smsd.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.ring... [javadoc] Loading source files for package org.openscience.cdk.smsd.tools... [javadoc] Constructing Javadoc information... [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest': net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest': net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest': net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest test-module: [echo] Running PMD checks for CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd test-module: [echo] Checking for unused code in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-unused test-module: [echo] Checking for migrating code updates in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-migrating BUILD SUCCESSFUL Total time: 33 seconds ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 09:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 09:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 09:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-08-28 18:54:09
|
Patches item #3323501, was opened at 2011-06-21 01:12 Message generated for change (Comment added) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Nobody/Anonymous (nobody) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- >Comment By: Egon Willighagen (egonw) Date: 2011-08-28 20:54 Message: Asad, it took some effort to rebase it on the current master branch, and the patch is almost 1MB large... I had a look at it, and have many requests. It's far from the quality of much of the core CDK code, and I honestly have no clue how to judge this patch. Code review does not work of patches of this size. I have attached my set of patches resulting from the rebase I did, and like you to verify if that looks good and nothing went wrong. I ran the 'qa-module' too and it finds quite a few JavaDoc and PMD problems. Please check the cdk/report/ folder for all output from this qa-module target. Asad, if I suggest to accept this patch for master, I will file a lot of bug reports, for all sorts of things. Will you promptly address those, with one patch for one bug report, and sending those in one by one, and not as another massive patch? I will file bug reports for JavaDoc and PMD issues, but also plan to file 'bug' reports for classes which seem to have been duplicated, like CDKRGraph; I see no need why it cannot use the CDK version... also, a class like HanserRingFinger seems it can move to another CDK package... Rajarshi, Christoph, are you happy with such a set up? Asad, additionally, please remove the output to STDOUT in the main source and in the test classes. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-07-26 13:23 Message: Status update of the patches after running the QA. ant -Dmodule=smsd qa-module Buildfile: /users/Asad/Software/GITROOT/cdksmsdgithub/build.xml checkPlatforms: check: dist.init: qa-module: [echo] Running QA for CDK's smsd module checkPlatforms: check: noJunit: test-module: [echo] Testing classes for CDK's smsd module. [echo] CDK dependencies defined: true [echo] Library dependencies defined: true [echo] Developer Library dependencies defined: true [junit] Running org.openscience.cdk.modulesuites.MsmsdTests [junit] Tests run: 222, Failures: 2, Errors: 0, Time elapsed: 8.498 sec [junit] Test org.openscience.cdk.modulesuites.MsmsdTests FAILED ojdcheck-module: [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/ojdcheck [javadoc] Generating Javadoc [javadoc] Javadoc execution [javadoc] Loading source files for package org.openscience.cdk.normalize... [javadoc] Loading source files for package org.openscience.cdk.smsd... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.matchers... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcgregor... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcsplus... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.rgraph... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.single... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.builder... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.map... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.query... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.substructure... [javadoc] Loading source files for package org.openscience.cdk.smsd.filters... [javadoc] Loading source files for package org.openscience.cdk.smsd.global... [javadoc] Loading source files for package org.openscience.cdk.smsd.helper... [javadoc] Loading source files for package org.openscience.cdk.smsd.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.ring... [javadoc] Loading source files for package org.openscience.cdk.smsd.tools... [javadoc] Constructing Javadoc information... [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest': net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest': net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest': net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest test-module: [echo] Running PMD checks for CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd test-module: [echo] Checking for unused code in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-unused test-module: [echo] Checking for migrating code updates in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-migrating BUILD SUCCESSFUL Total time: 33 seconds ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 10:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-08-28 18:56:41
|
Patches item #3323501, was opened at 2011-06-21 01:12 Message generated for change (Settings changed) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master >Group: Needs Revision Status: Open Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) >Assigned to: Asad (asadrahman) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-08-28 20:54 Message: Asad, it took some effort to rebase it on the current master branch, and the patch is almost 1MB large... I had a look at it, and have many requests. It's far from the quality of much of the core CDK code, and I honestly have no clue how to judge this patch. Code review does not work of patches of this size. I have attached my set of patches resulting from the rebase I did, and like you to verify if that looks good and nothing went wrong. I ran the 'qa-module' too and it finds quite a few JavaDoc and PMD problems. Please check the cdk/report/ folder for all output from this qa-module target. Asad, if I suggest to accept this patch for master, I will file a lot of bug reports, for all sorts of things. Will you promptly address those, with one patch for one bug report, and sending those in one by one, and not as another massive patch? I will file bug reports for JavaDoc and PMD issues, but also plan to file 'bug' reports for classes which seem to have been duplicated, like CDKRGraph; I see no need why it cannot use the CDK version... also, a class like HanserRingFinger seems it can move to another CDK package... Rajarshi, Christoph, are you happy with such a set up? Asad, additionally, please remove the output to STDOUT in the main source and in the test classes. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-07-26 13:23 Message: Status update of the patches after running the QA. ant -Dmodule=smsd qa-module Buildfile: /users/Asad/Software/GITROOT/cdksmsdgithub/build.xml checkPlatforms: check: dist.init: qa-module: [echo] Running QA for CDK's smsd module checkPlatforms: check: noJunit: test-module: [echo] Testing classes for CDK's smsd module. [echo] CDK dependencies defined: true [echo] Library dependencies defined: true [echo] Developer Library dependencies defined: true [junit] Running org.openscience.cdk.modulesuites.MsmsdTests [junit] Tests run: 222, Failures: 2, Errors: 0, Time elapsed: 8.498 sec [junit] Test org.openscience.cdk.modulesuites.MsmsdTests FAILED ojdcheck-module: [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/ojdcheck [javadoc] Generating Javadoc [javadoc] Javadoc execution [javadoc] Loading source files for package org.openscience.cdk.normalize... [javadoc] Loading source files for package org.openscience.cdk.smsd... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.matchers... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcgregor... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcsplus... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.rgraph... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.single... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.builder... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.map... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.query... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.substructure... [javadoc] Loading source files for package org.openscience.cdk.smsd.filters... [javadoc] Loading source files for package org.openscience.cdk.smsd.global... [javadoc] Loading source files for package org.openscience.cdk.smsd.helper... [javadoc] Loading source files for package org.openscience.cdk.smsd.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.ring... [javadoc] Loading source files for package org.openscience.cdk.smsd.tools... [javadoc] Constructing Javadoc information... [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest': net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest': net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest': net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest test-module: [echo] Running PMD checks for CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd test-module: [echo] Checking for unused code in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-unused test-module: [echo] Checking for migrating code updates in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-migrating BUILD SUCCESSFUL Total time: 33 seconds ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 10:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 10:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-09-14 12:47:39
|
Patches item #3323501, was opened at 2011-06-20 23:12 Message generated for change (Comment added) made by gilleain You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: Needs Revision >Status: Closed Resolution: None Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Asad (asadrahman) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- >Comment By: gilleain maclean torrance (gilleain) Date: 2011-09-14 12:47 Message: Reluctantly, this patch should really be rejected in favour of making smaller, separate patches on topics (like a), b), c)). I will use Egon's patches to guide this process, and I see that some of them are javadoc fixes, so hopefully those will apply directly in the topic branch. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-08-28 18:54 Message: Asad, it took some effort to rebase it on the current master branch, and the patch is almost 1MB large... I had a look at it, and have many requests. It's far from the quality of much of the core CDK code, and I honestly have no clue how to judge this patch. Code review does not work of patches of this size. I have attached my set of patches resulting from the rebase I did, and like you to verify if that looks good and nothing went wrong. I ran the 'qa-module' too and it finds quite a few JavaDoc and PMD problems. Please check the cdk/report/ folder for all output from this qa-module target. Asad, if I suggest to accept this patch for master, I will file a lot of bug reports, for all sorts of things. Will you promptly address those, with one patch for one bug report, and sending those in one by one, and not as another massive patch? I will file bug reports for JavaDoc and PMD issues, but also plan to file 'bug' reports for classes which seem to have been duplicated, like CDKRGraph; I see no need why it cannot use the CDK version... also, a class like HanserRingFinger seems it can move to another CDK package... Rajarshi, Christoph, are you happy with such a set up? Asad, additionally, please remove the output to STDOUT in the main source and in the test classes. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-07-26 11:23 Message: Status update of the patches after running the QA. ant -Dmodule=smsd qa-module Buildfile: /users/Asad/Software/GITROOT/cdksmsdgithub/build.xml checkPlatforms: check: dist.init: qa-module: [echo] Running QA for CDK's smsd module checkPlatforms: check: noJunit: test-module: [echo] Testing classes for CDK's smsd module. [echo] CDK dependencies defined: true [echo] Library dependencies defined: true [echo] Developer Library dependencies defined: true [junit] Running org.openscience.cdk.modulesuites.MsmsdTests [junit] Tests run: 222, Failures: 2, Errors: 0, Time elapsed: 8.498 sec [junit] Test org.openscience.cdk.modulesuites.MsmsdTests FAILED ojdcheck-module: [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/ojdcheck [javadoc] Generating Javadoc [javadoc] Javadoc execution [javadoc] Loading source files for package org.openscience.cdk.normalize... [javadoc] Loading source files for package org.openscience.cdk.smsd... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.matchers... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcgregor... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcsplus... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.rgraph... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.single... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.builder... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.map... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.query... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.substructure... [javadoc] Loading source files for package org.openscience.cdk.smsd.filters... [javadoc] Loading source files for package org.openscience.cdk.smsd.global... [javadoc] Loading source files for package org.openscience.cdk.smsd.helper... [javadoc] Loading source files for package org.openscience.cdk.smsd.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.ring... [javadoc] Loading source files for package org.openscience.cdk.smsd.tools... [javadoc] Constructing Javadoc information... [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest': net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest': net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest': net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest test-module: [echo] Running PMD checks for CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd test-module: [echo] Checking for unused code in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-unused test-module: [echo] Checking for migrating code updates in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-migrating BUILD SUCCESSFUL Total time: 33 seconds ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 08:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 08:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 08:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |
From: SourceForge.net <no...@so...> - 2011-09-14 12:47:58
|
Patches item #3323501, was opened at 2011-06-20 23:12 Message generated for change (Settings changed) made by gilleain You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: Needs Revision Status: Closed >Resolution: Rejected Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Asad (asadrahman) Summary: SMSD patch Initial Comment: SMSD patch with latest code a) Isomorphism b) Substructure search c) Thread safe ---------------------------------------------------------------------- Comment By: gilleain maclean torrance (gilleain) Date: 2011-09-14 12:47 Message: Reluctantly, this patch should really be rejected in favour of making smaller, separate patches on topics (like a), b), c)). I will use Egon's patches to guide this process, and I see that some of them are javadoc fixes, so hopefully those will apply directly in the topic branch. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-08-28 18:54 Message: Asad, it took some effort to rebase it on the current master branch, and the patch is almost 1MB large... I had a look at it, and have many requests. It's far from the quality of much of the core CDK code, and I honestly have no clue how to judge this patch. Code review does not work of patches of this size. I have attached my set of patches resulting from the rebase I did, and like you to verify if that looks good and nothing went wrong. I ran the 'qa-module' too and it finds quite a few JavaDoc and PMD problems. Please check the cdk/report/ folder for all output from this qa-module target. Asad, if I suggest to accept this patch for master, I will file a lot of bug reports, for all sorts of things. Will you promptly address those, with one patch for one bug report, and sending those in one by one, and not as another massive patch? I will file bug reports for JavaDoc and PMD issues, but also plan to file 'bug' reports for classes which seem to have been duplicated, like CDKRGraph; I see no need why it cannot use the CDK version... also, a class like HanserRingFinger seems it can move to another CDK package... Rajarshi, Christoph, are you happy with such a set up? Asad, additionally, please remove the output to STDOUT in the main source and in the test classes. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-07-26 11:23 Message: Status update of the patches after running the QA. ant -Dmodule=smsd qa-module Buildfile: /users/Asad/Software/GITROOT/cdksmsdgithub/build.xml checkPlatforms: check: dist.init: qa-module: [echo] Running QA for CDK's smsd module checkPlatforms: check: noJunit: test-module: [echo] Testing classes for CDK's smsd module. [echo] CDK dependencies defined: true [echo] Library dependencies defined: true [echo] Developer Library dependencies defined: true [junit] Running org.openscience.cdk.modulesuites.MsmsdTests [junit] Tests run: 222, Failures: 2, Errors: 0, Time elapsed: 8.498 sec [junit] Test org.openscience.cdk.modulesuites.MsmsdTests FAILED ojdcheck-module: [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/ojdcheck [javadoc] Generating Javadoc [javadoc] Javadoc execution [javadoc] Loading source files for package org.openscience.cdk.normalize... [javadoc] Loading source files for package org.openscience.cdk.smsd... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.matchers... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcgregor... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.mcsplus... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.rgraph... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.single... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.builder... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.map... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.query... [javadoc] Loading source files for package org.openscience.cdk.smsd.algorithm.vflib.substructure... [javadoc] Loading source files for package org.openscience.cdk.smsd.filters... [javadoc] Loading source files for package org.openscience.cdk.smsd.global... [javadoc] Loading source files for package org.openscience.cdk.smsd.helper... [javadoc] Loading source files for package org.openscience.cdk.smsd.interfaces... [javadoc] Loading source files for package org.openscience.cdk.smsd.ring... [javadoc] Loading source files for package org.openscience.cdk.smsd.tools... [javadoc] Constructing Javadoc information... [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest': net.sf.cdk.tools.checkdoctest.MissingGithashTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest': net.sf.cdk.tools.checkdoctest.MissingModuleTagletTest [javadoc] Could not load class 'net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest': net.sf.cdk.tools.checkdoctest.IncorrectBugNumberTagletTest test-module: [echo] Running PMD checks for CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd test-module: [echo] Checking for unused code in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-unused test-module: [echo] Checking for migrating code updates in CDK's smsd module. [mkdir] Created dir: /users/Asad/Software/GITROOT/cdksmsdgithub/reports/pmd-migrating BUILD SUCCESSFUL Total time: 33 seconds ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 08:42 Message: ant -Dmodule=smsd qa-module ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2011-06-21 08:34 Message: Hi Egon, How to run CDK QA system ? Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-06-21 08:27 Message: Asad, this is a massive patch. Just in the first 4% I see numerous problems. Have you run the CDK QA system against it? Please do. I know this is painful work, but we all have to do it. The current patch has way to much I would comment on, for me to review this patch in the next weeks. Solving all OpenJavaDocCheck, PMD, JUnit (coverage) problem will very much help. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3323501&group_id=20024 |