Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[coinex] Extend functionality #4884

Merged
merged 14 commits into from
Jun 5, 2024
Merged

[coinex] Extend functionality #4884

merged 14 commits into from
Jun 5, 2024

Conversation

bigscoop
Copy link
Contributor

  • Added symtol mappings
  • Added exchange metadata initialization
  • Added error mapping
  • Added getting of chain infos
  • Added getting of orderbook
  • Added getting of account balance
  • Added getting of tickers
  • Added params digest
  • Added http requests
  • Added integration tests

@@ -388,7 +394,6 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>${version.slf4j}</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slf4j should be in dependencies, not in dependency management. That way it's in all submodules without needed to explicitly add it in each child module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you're right, but it has to be in both sections:
in dependencyManagement we define the version without declaring the factual dependency
in dependencies we declare the factual dependency and the version is taken the one that is declared in dependencyManagement

The advantage of that approach is that we avoid compiling against one version and using another in the runtime.

Now it looks like that: we compile against slf4j-api 2.0.13, but effectively getting 2.0.6 via transitive dependency:

[INFO] +- org.knowm.xchange:xchange-core:jar:5.1.2-SNAPSHOT:compile
[INFO] |  +- com.github.mmazi:rescu:jar:3.0:compile (version managed from 3.0)
[INFO] |  |  +- (org.slf4j:slf4j-api:jar:2.0.6:compile - omitted for conflict with 2.0.13)

Once we declare the version in dependencyManagement the dependency tree looks way better - we compile against 2.0.13 and get 2.0.13:

[INFO] +- org.knowm.xchange:xchange-core:jar:5.1.2-SNAPSHOT:compile
[INFO] |  +- com.github.mmazi:rescu:jar:3.0:compile (version managed from 3.0)
[INFO] |  |  +- (org.slf4j:slf4j-api:jar:2.0.13:compile - version managed from 2.0.6; omitted for duplicate)

That is a well known problem caled dependency hell and there is a maven-enforcer-plugin that helps against such problems.
If we agree on that I'll do the analysis for all modules and can prepare the PR afterwards.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool. I didn't even know this issue existed at all.

@timmolter timmolter merged commit f9d72ec into knowm:develop Jun 5, 2024
1 check passed
@bigscoop bigscoop deleted the coinex branch June 5, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants