-
Notifications
You must be signed in to change notification settings - Fork 11
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
Setup solana liquidity pool checker tool #16
base: master
Are you sure you want to change the base?
Setup solana liquidity pool checker tool #16
Conversation
27a0d7b
to
c21c093
Compare
Thank you! Review should be resolved soon then will merge (once duplicate library topic is addressed). Some tests would be helpful. |
@@ -78,6 +78,24 @@ | |||
<version>2.22.1</version> | |||
</dependency> | |||
|
|||
<dependency> | |||
<groupId>org.apache.httpcomponents.client5</groupId> | |||
<artifactId>httpclient5</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SolanaJ already provides OkHttp which has proven to be a strong HTTP library. I'm slightly averse to bundling yet another HTTP library (Apache HTTPClient). I'd be interested in hearing reasoning for Apache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with apache because I worked with it in the past. Not much reasoning. I have not worked with OkHttp, but I will update the code to use that instead of apache lib.
pom.xml
Outdated
@@ -144,8 +162,8 @@ | |||
<goal>sign</goal> | |||
</goals> | |||
<configuration> | |||
<homedir>c:/Users/Michael/.gnupg/</homedir> | |||
<keyname>0x27FAE7D2</keyname> | |||
<homedir>/Users/chintan_mbp/.gnupg/</homedir> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can move the maven-gpg-plugin
to only run during the deploy
job, thus removing the GPG signing requirement for non-deploys. See https://github.com/skynetcap/solanaj/blob/main/pom.xml#L198 for reference. So far I will be the only one deploying, so can leave the paths as they were, once it's moved to the deploy profile.
i.e.
<profiles>
<profile>
<id>deploy</id>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-gpg-plugin</artifactId>
<executions>
<execution>
<id>sign-artifacts</id>
<phase>verify</phase>
<goals>
<goal>sign</goal>
</goals>
<configuration>
<homedir>c:/Users/Michael/.gnupg/</homedir>
<keyname>0x27FAE7D2</keyname>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: I went ahead and did this myself.
return tokenInfoGetRequest; | ||
} | ||
|
||
private static HttpClientResponseHandler<JsonNode> getHttpRequestHandler() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned earlier, would prefer to use OkHttp as a single HTTP library. If you don't have bandwidth for those modifications, I can possibly just merge it and handle the tech debt later. Let me know your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry late reply. If not yet done, I will work on this week.
Ready to merge to the owner repo
This change adds the raydium module and the ability to check if the raydium LP has been burned and locked.