#263 Missing SMSD 1.2.0 patches

Needs_Review
closed
Asad
master (162)
5
2012-10-28
2010-08-30
Asad
No

The first commit and changes in the CDK has lead to changes in the SMSD lib.
I am attaching the updated code/missing patches for review.

Thanks

Asad

Discussion

  • Egon Willighagen

    From IRC:

    [08/30/10 13:43] <egonw> s9asad: patch 0001: the commit message is incorrect
    [08/30/10 13:43] <egonw> it does not just add IMatcher, it also removes many files... ??
    [08/30/10 13:44] <egonw> e.g. it remov Isomorphism.java which is added by 0002 again ??
    [08/30/10 13:44] <egonw> 0003: new methods in this patch do not have JavaDoc
    [08/30/10 13:46] <egonw> 0004: you could split up that patch into: 0004a: remove outcommented code; 0004b: updated for change in index API: what used to be null is now reflected as -1 (which I guess what is going on)
    [08/30/10 13:47] <egonw> 0005: you import the wrong Assert class... use the org.junit.Assert instead
    [08/30/10 13:48] <egonw> "a)constructor call fixed" -> bad commit message... instead, describe what was wrong, and how you fixed it
    [08/30/10 13:49] <egonw> (that was 0006)
    [08/30/10 13:50] <egonw> 0007: there is no need to add empty methods for @BeforeClass, @AfterClass, and @After ... empty constructors are not needed either
    [08/30/10 13:51] <egonw> oh.... and 0001 does not even add an IMatcher interface as promised in the commit message

     
  • Asad

    Asad - 2010-08-30

    I have updated the patches as advised.

     
  • Asad

    Asad - 2010-08-30

    cleaner patch

     
  • Egon Willighagen

    OK, the patches are much more clean now. I can now actually understand what has changed.

    There are problems with the patches, which I'll ask you to fix later:

    • "Default constructor added" is not a proper JavaDoc... JavaDoc is not for developer documentation
    • there is JavaDoc lacking here and there

    (please check the Nightly reports for the SMSD module and try to fix some of them... perhaps ask people to help you with that... these can be good junior jobs (see the special tracker))

     
  • Asad

    Asad - 2010-08-30

    OK, will try and update the docs in the next version/submission.

     
  • Egon Willighagen

    Applied to master.

     

Log in to post a comment.