-
Notifications
You must be signed in to change notification settings - Fork 79
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
chore(public): Improve Universal Router README with clearer instructions and example #119
base: main
Are you sure you want to change the base?
Conversation
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.
thank you for the PR! left some comments
sdks/universal-router-sdk/README.md
Outdated
import { Trade as V2TradeSDK } from '@uniswap/v2-sdk' | ||
import { Trade as V3TradeSDK } from '@uniswap/v3-sdk' | ||
import { MixedRouteTrade, MixedRouteSDK, Trade as RouterTrade } from '@uniswap/router-sdk' | ||
import { Trade as V2TradeSDK } from '@uniswap/v2-sdk' |
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.
duplicated import
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.
ooops. thanks!
sdks/universal-router-sdk/README.md
Outdated
// const txResponse = await wallet.sendTransaction(tx) | ||
// await txResponse.wait() | ||
|
||
console.log('Transaction parameters:', tx) |
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.
nit: rm
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.
removed the console.log
sdks/universal-router-sdk/README.md
Outdated
// Send the transaction using your preferred method (e.g., ethers.js, web3.js) | ||
// const txResponse = await wallet.sendTransaction(tx) | ||
// await txResponse.wait() |
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.
fine to uncomment this in my opinion!
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.
uncommented
sdks/universal-router-sdk/README.md
Outdated
This example demonstrates how to: | ||
1. Import necessary classes and functions from Uniswap SDKs | ||
2. Define tokens for the trade | ||
3. Create a trade object (simplified in this example) | ||
4. Set up trade options including slippage tolerance and recipient | ||
5. Generate the calldata and value for the swap | ||
6. Prepare a transaction object for sending to the Universal Router | ||
|
||
Remember to handle errors, validate inputs, and implement proper transaction sending and confirmation in your actual implementation. |
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.
can simplify this to:
The code above is provided strictly as an example implementation of using the SDK to build a UniversalRouter swap.
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.
updated. thx!
sdks/universal-router-sdk/README.md
Outdated
## Contributing | ||
Contributions to improve the SDK or documentation are welcome. Please ensure you follow the existing code style and add appropriate tests for any new functionality. |
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.
remove, we can create a separate CONTRIBUTING.md in the future
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.
removed.
Description
This PR updates the README of the universal-router-sdk to provide clearer and more comprehensive installation instructions. The changes include:
swapERC20CallParameters()
These changes aim to make it easier for new users to get started with the SDK
How Has This Been Tested?
The changes in this PR are documentation updates and do not affect the codebase. However, I have:
Are there any breaking changes?
No, there are no breaking changes. This PR only updates the README file.
Feedback Focus
I'd appreciate feedback on:
Follow Ups
Potential follow-up tasks could include: