DO NOT MERGE (For discussion only) Merging XChange-Stream with XChange. Addresses #3220

[TSavo]

Per the discussion in #3220 XChange-Stream has been hard at work getting the project ready for merge with XChange. They've made a point to:

• Remove all compiler warnings
• Reformat all the code using an automated code formatter
• Rename all packages to xchange-stream-*
• Separate all the Unit Tests from Integration tests
• Make a call to close out existing PRs

This is meant to be an EXAMPLE of what a possible merge scenario might look like. It takes the latest from development of both projects and merges them together by:

• Reparenting all child pom.xmls to xchange-parent
• Refactoring info.bitrich.xchangestream package to org.knowm.xchange.stream
• Adding Names and Descriptions to all stream pom.xmls
• Adjusting dependencies to include new dependencies shared across projects

It's meant to be an example of what a merge would look like. It does NOT imply that the two projects are ready to merge, but for brave souls who wish to see what one possible future might look like, this provides a fully functional example of the two projects working in harmony and allows for early beta testers to find possible issues, and discuss how the actual merge might be different.

What hasn't been done:

• README.md merge
• <Your discussion is needed here! Please comment on what else should be done or what should be done differently.>

I'll do my best to keep both development branches refreshed from HEAD, let the discussion commence!

[timmolter]

should we instead name all these modules like xchange-okcoin-stream instead? That way they will end up in our IDEs grouped together better, i.e. xchange-core will be next to xchange-core-stream.

[badgerwithagun]

@timmolter We've already renamed using the xchange-stream prefix. Can we perhaps aim for a merge first and then rename prior to the next XChange release? That way I can try and put together the merge sooner rather than later (there are quite a few people screaming for it ;))

[timmolter]

yeah, we can do the merge first like you suggest. No problem.

[badgerwithagun]

I was about to come on here and propose a merge approach. The main problem with this one is that it loses the change history from the original project.

I was going to suggest something like the following, inspired by this article.

• Shut down PR submissions for the xchange-stream project and update the README explaining what's going on.
• Create a branch in a local clone of xchange-stream (let's call it premerge)
  • Delete all artifacts in the root directory which we don't want to move over to XChange (e.g. the parent POM) or which clash names.
  • Push that branch to github.
• On a local clone of knowm/XChange:
  • Create a branch (let's call it stream-merge)
  • Add bitrich-info/xchange-stream as a remote and fetch.
  • Merge github/xchange-stream:premerge into the local branch. This will bring the entire history of xchange-stream with it.
  • Modify the parent POM and travis build to build the new stream modules too.
• Push the branch and make sure the build runs green
• Merge the branch into master.

Trying to do this via PRs will be be a nightmare, I think.

CC @dozd

[dozd]

Definitively, do this outside PR is advised. Your approach seems working & good to me.

[earce]

Hey, I'm following this effort and wanted to see if there something I can look at to see how far along this is or to get a sense of the time this is expected to take.

[badgerwithagun]

@walec51 and @timmolter

Would you mind having a look at the xchange-stream code, then give me a laundry list of anything you'd like resolved prior to my handing you the branch for merge? I'd like to get this done soon as there are quite a few people clamouring for it

@earce I'm going to try and do this shortly. Just need to co-ordinate with the guys here.

[timmolter]

Sorry for not replying to this earlier! The code looks good to me, assuming it works! :) I'll do my best trying to merge the two projects following your instructions above. Let me know when I should proceed. Thanks!

[badgerwithagun]

Fab. @timmolter , @walec51 , @dozd and all following this thread: I'm going to start working on this now. I've updated the README in the other project and will be posting on the open pull requests with instructions as I go.

[sowhynot]

great initiative, round of applause folks

[earce]

Everyone thanks for looking at this, super excited to see this come together.

[badgerwithagun]

Well, that was easier than I expected for a first iteration @timmolter .

How does this look as a first pass? https://github.com/badgerwithagun/XChange/tree/stream-merge

[timmolter]

@badgerwithagun I just accepted some PRs on the XChange develop branch. Could you sync up your branch please? Make sure you're extending the develop branch and not master. Thanks for your efforts!

[badgerwithagun]

@timmolter I've just merged knowm/develop into my branch.

[badgerwithagun]

For completeness, I'm going to try and extend the documentation to cover all the streaming stuff. Some of this will be in the README and some in the Wiki. Will take a little while though. I'll do it as I work through closing all the open issues and PRs on the outgoing project.

[timmolter]

Just some notes as I do this merge.

1. log4j is used for the slf4j impl and not logback like in XChange. We're gonna need to pick one. Maybe even slf4j-simple?

[timmolter]

This dependency in kraken seems to be an odd-ball. Can it be refactored out?

        <dependency>
            <groupId>commons-beanutils</groupId>
            <artifactId>commons-beanutils</artifactId>
            <version>1.9.4</version>
        </dependency>

[timmolter]

Here's another odd-ball in hitbtc:

       <dependency>
            <groupId>com.jayway.jsonpath</groupId>
            <artifactId>json-path-assert</artifactId>
            <version>2.4.0</version>
            <scope>test</scope>
        </dependency>

[timmolter]

The .gitignore file needs to have all the streaming stuff added back. Now's a good time to carefully re-look at everything.

[badgerwithagun]

Good spots, thanks - I'll take a look at those dependencies. I thought I'd got rid of log4j but I suspect there are some rogue dependencies hidden in there still. Will have a look this evening. I just assumed we'd use logback across the board.

[badgerwithagun]

@timmolter I can't see any log4j remaining dependencies in the added streaming code. Unless my IDE's search is letting me down...

[badgerwithagun]

The json-path-assert dependency is test-only. Seems like a handy utility and doesn't affect runtime. Worth worrying about?

[timmolter]

I'm trying to get this merge done by the end of today. Don't worry about updating anything until I get it done.

[badgerwithagun]

commons-beanutils could be pretty easily removed by the looks of things. It's just being used to parse BigDecimal and Doubles from a string.

[badgerwithagun]

I'm trying to get this merge done by the end of today. Don't worry about updating anything until I get it done.

Gotcha

[timmolter]

Another odd-ball in bitfinex:

      <dependency>
        <groupId>javax.xml.bind</groupId>
        <artifactId>jaxb-api</artifactId>
        <version>2.3.1</version>
      </dependency>

It's used only for one method in BitfinexStreamingService:

signature = DatatypeConverter.printHexBinary(result);

[badgerwithagun]

That one is needed for Java 9+ compatibility in a number of places, I believe, and is in dependency management in the parent POM. Can probably just remove the version.

[timmolter]

weird, I definitely had to add it to the parent pom and I think it's just only for that. Could have missed something though I guess.

[timmolter]

OK, it's merged into XChange/develop and travis ci is running it. Fingers crossed. It built for me locally.

[timmolter]

Is there any way to visualize the git commit graph? I think it'd be neat to see.

[timmolter]

OK Travis CI passed. An we're now on 5.0.0-SNAPSHOT. We can start accepting PRs again!