Menu
▾
▴
[Cdk-devel] [ cdk-Patches-2987698 ] SMSD CDK patch
|
From: SourceForge.net <no...@so...> - 2010-04-28 14:45:50
|
Patches item #2987698, was opened at 2010-04-15 15:46 Message generated for change (Settings changed) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2987698&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: Accepted >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Asad (asadrahman) Assigned to: Egon Willighagen (egonw) Summary: SMSD CDK patch Initial Comment: I would request to integrate the SMSD with the CDK. The nightly build of the code is available http://pele.farmbio.uu.se/nightly-smsd/ Kind regards Asad ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-04-23 18:59 Message: Well, the smsdNew patches made a lot of changes to the core data classes... I think that was mostly whitespace and is already cleaned from the SMSD patch... the list of files I just gave are other example of files of which it is rather unlikely the SMSD patch should touch them... the CML reading code was changed too, but that is removed from the rebased set too... For example, why are the files in the org.openscience.cdk.tools.diff packages changed? I guess it's whitespace again, but want to make sure... Can we chat later about this more? I like to suggest to 'collapse' the patch into a single commit... that will make the commit a lot cleaner, and do some last fixing after that... e.g. saw another instance of {@cdk.cite} not used... ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2010-04-23 18:18 Message: Sender: asadrahman Hi Egon, Now that you have come up with a list of altered classes, could you please be a little more precise about the changes which do not fit into the CDK -SMSD framework? It would be really interesting to note. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2010-04-23 18:12 Message: Hi Egon, Now that you have come up with a list which you feel is altered. Could you be little more precise what are the changes you think has be incurred the CDK -SMSD brach which doesn't fit the framework? It would be really interesting to note. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-04-23 17:32 Message: That list is after removing a few code clean up patches... and rebasing with ignoring whitespace changes. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-04-23 17:31 Message: The following files outside the smsd/ package are modified: --- a/src/main/org/openscience/cdk/EnzymeResidueLocator.java --- a/src/main/org/openscience/cdk/isomorphism/IsomorphismTester.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/AromaticAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/AromaticQueryBond.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/HybridizationNumberAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/ImplicitHCountAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/LogicalOperatorAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/LogicalOperatorBond.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/MassAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/NonCHHeavyAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/OrderQueryBond.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/PeriodicGroupNumberAtom.java --- a/src/main/org/openscience/cdk/isomorphism/matchers/smarts/RingAtom.java --- a/src/main/org/openscience/cdk/isomorphism/mcss/RNode.java --- a/src/main/org/openscience/cdk/tools/BremserOneSphereHOSECodePredictor.java --- a/src/main/org/openscience/cdk/tools/CDKUtilities.java --- a/src/main/org/openscience/cdk/tools/diff/AtomContainerDiff.java --- a/src/main/org/openscience/cdk/tools/diff/AtomDiff.java --- a/src/main/org/openscience/cdk/tools/diff/AtomTypeDiff.java --- a/src/main/org/openscience/cdk/tools/diff/BondDiff.java --- a/src/main/org/openscience/cdk/tools/diff/ChemObjectDiff.java --- a/src/main/org/openscience/cdk/tools/diff/ElectronContainerDiff.java --- a/src/main/org/openscience/cdk/tools/diff/ElementDiff.java --- a/src/main/org/openscience/cdk/tools/diff/IsotopeDiff.java --- a/src/main/org/openscience/cdk/tools/diff/LonePairDiff.java --- a/src/main/org/openscience/cdk/tools/diff/SingleElectronDiff.java --- a/src/main/org/openscience/cdk/tools/diff/tree/AbstractDifference.java --- a/src/main/org/openscience/cdk/tools/diff/tree/AbstractDifferenceList.java --- a/src/test/org/openscience/cdk/isomorphism/IsomorphismTesterTest.java ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-04-23 13:24 Message: I'm in the process of rebasing the 524! patches on master, and now see that several patches touch code outside the scope of the SMSD functionality. That is really bad practice. I'm going to block this patch for now, until I am sure no patch fiddles with code under my supervision or written by me. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2010-04-22 20:28 Message: >From my perspective, there are a few minor things to do, and a number of code clean ups which we can address later. But, the main blocker is a needed rebase on the latest master, which I'll do asap. ---------------------------------------------------------------------- Comment By: Asad (asadrahman) Date: 2010-04-20 14:41 Message: Dear Chris, Thank you very much. I have fixed the code example in the java doc (thanks to Egon). kind regards Asad ---------------------------------------------------------------------- Comment By: Christoph Steinbeck (steinbeck) Date: 2010-04-19 13:26 Message: I have reviewed the current state of the patch in term of PMD problems, Unit test coverage and working/failing tests as well as JavaDoc quality. I'm happy with the code quality as far as one can assess them in a short 1-hour review and I think the unit test coverage is good. None of the unit tests fail because of SMSD problems. Two tests fail because of known upstream CDK problems. The code examples added by Asad to the Javadoc do not show up in the HTML version and I asked Asad to rectify this. Ones this is done, I recommend the patch for inclusion in the CDK. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2987698&group_id=20024 |