final steps

[uli-f]

It feels to me like we are mostly done.

Once the open PRs are merged and the two issues #16 and #17 are closed I would like to do some final testing on my end by putting a high volume of reactions through the reaction-to-rinchi conversion and the rinchi-to-reaction conversion.

I'll report back here on any results regarding these tests.

[ntk73]

@uli-f , rinchi branch is 128 commits ahead](https://github.com/ideaconsult/cdk/compare/cdk:cdk:main...rinchi), 102 commits behind (https://github.com/ideaconsult/cdk/compare/rinchi...cdk:cdk:main) cdk:main.

I suppose that you are aware that in September CDK migrated to 2.9-SNAPSHOT as well as CDK migrated to JUnit5 (I saw that you have some commits in CDK recently :-) ). So we will need to do some changes in the pom and may be a lot of refactoring on all tests before pull request.

I have synchronized the master branch in the Ideaconsult repo but rinchi branch is still behind (I do not want to disrupt the current work). How big will be the new testing code you are planning to add? My question is whether to migrate before or after your test code additions?

[uli-f]

So we will need to do some changes in the pom and may be a lot of refactoring on all tests before pull request.

I talked to John and he assumed that there won't be too many changes necessary when re-basing the rinchi branch to main. It will probably mostly be about the JUnit migration.

How big will be the new testing code you are planning to add?

I probably did not make that very clear: I don't plan on adding any tests. My intention is to take a biggish number of reactions and use them as an input for a round-trip computation (reaction-to-rinchi conversion -> rinchi-to-reaction). If I experience any exceptions I'll open an issue here.

My question is whether to migrate before or after your test code additions?

Please go ahead and re-base. Let me know if you need any help.

OT: I opened five small PRs in jna-inchi. Once you merged them we can open a PR for the rinchi branch in jna-inchi.

[ntk73]

@uli-f , I have rebased the rinchi branch to cdk:master branch. The cdk-rinchi is migrated to 2.9-SNAPSHOT and JUnit 5 testing framework. Actually the latter caused a lot of manual code refactoring, since in JUnit 5, they decided to change the input parameters order of all assertYYY() functions. More specifically the message string is moved from the front (first parameter) to the end (to be the last input parameter). For example: Assert.assertEquals("RInChI status: ", Status.SUCCESS, gen.getRInChIStatus()); is changed to: Assertions.assertEquals(Status.SUCCESS, gen.getRInChIStatus(), "RInChI status: ");

This should explain the large amount of line changes in commit 9ea3ad7a4ed8dfa19d4d7e4643751bdf42abe131.

Also, just to mention, that I had some minor problem with the Eclipse workspace running all the new JUnit 5 tests (I did not recognize initially, that it stored in the test run configs the old JUnit 4 framework). I suppose that this is just a small issue with the Eclipse IDE config and have nothing to do with the maven config, but have it in mind just for any case, and if you come across on a problem, we will search a solution (you could give feedback from the IntelliJ ...)

Finally, and also important, concerning some git issues: Due to the large number of commits for rebasing, after rebasing the rinchi branch and making necessary changes with latest commit (9ea3ad7a4ed8dfa19d4d7e4643751bdf42abe131), the push had to performed with command: git push --force. On the other computer I needed either to get rinchi branch a new, or to run commands: git fetch git reset --hard origin/rinchi I am writing this, because you might come across to some specifics on the syncing your fork with the latest version of rinchi branch from our repo. I suppose the synchronizing of your fork from the web interface of git should work without a problem. If you have problems with syncing or git pull afterwards, we will consult @kerberizer

[uli-f]

Thank you Nikolay for putting in the work to re-base the rinchi branch.

Also, just to mention, that I had some minor problem with the Eclipse workspace running all the new JUnit 5 tests (I did not recognize initially, that it stored in the test run configs the old JUnit 4 framework). I suppose that this is just a small issue with the Eclipse IDE config and have nothing to do with the maven config, but have it in mind just for any case, and if you come across on a problem, we will search a solution (you could give feedback from the IntelliJ ...)

I did not have any issues with Intellij. Did a maven clean compile test and everything worked fine.

Finally, and also important, concerning some git issues:

Thanks a lot for mentioning this! I did experience git issues when updating the rinchi branch, so I just removed my local copy of it and checked it out again from the ideaconsult/cdk repo. This worked fine.

[uli-f]

What's left now to do from my point of view is a final clean up (indentations, javadocs).

I'll take care of this once the open PRs are merged.

[kerberizer]

Thanks a lot for mentioning this! I did experience git issues when updating the rinchi branch, so I just removed my local copy of it and checked it out again from the ideaconsult/cdk repo. This worked fine.

Best way to do it in such cases, indeed. :)

[uli-f]

What's left now to do from my point of view is a final clean up (indentations, javadocs).

I'll take care of this once the open PRs are merged.

Opened PR #28

[uli-f]

My understanding is that we have to wait to open a PR for the RInChI functionality in CDK until the PR for jna-inchi has been merged and published on the public maven repo so that CDK can update the dependency on jna-inchi to the latest version.

@ntk73 Daniel seems to be most active on the weekends so it would be great if you can open that PR for jna-inchi until Friday.

[ntk73]

Opened PR #28

PR #28 is merged