-
Notifications
You must be signed in to change notification settings - Fork 8
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(doc): Add comments and derive attributes #60
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.
Do not base the doc changes on the big-decimal
changes as it changes the behaviour of this SDK. The amount of comments is excessive especially in the addresses
mod since most of the variables are self-explanatory.
/// Contains commonly used items from the Uniswap SDK Core. | ||
/// | ||
/// This module re-exports items that are commonly used together, | ||
/// making it easier to import them in other parts of your application. |
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.
These comments should be placed on the top of each module.
src/entities/fractions/mod.rs
Outdated
/// This module contains functionality related to currency amounts in the context of fractions. | ||
pub mod currency_amount; | ||
/// This module contains functionality related to fractions, which are used for various calculations | ||
/// in the SDK. | ||
pub mod fraction; | ||
/// This module contains functionality related to percentages, which are used for expressing | ||
/// fractions in a more human-readable format. | ||
pub mod percent; | ||
/// This module contains functionality related to prices, which are used for calculating exchange | ||
/// rates and other financial metrics. |
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.
These comments are so AI that they are too general and don't provide much insight.
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.
true, will still have to change them just wanted something to fill the comments
tbh I didn't do it myself, used phind to generate most of the comments |
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 don't mean to waste your time. But could you start a new PR and only add comments where the code is not self-explanatory?
sure, point me to the parts |
Use your judgement. Break down the changes. |
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 I said, start a new PR and only add comments where the code isn't self-explanatory. Do not fill the code with useless AI comments to reduce readability. Move the hash map change to a different PR.
yeah true, but there would have to be documentation though even for the self explanatory ones(from alloy) and can it not just be possible to just do both in one pr and maybe just change the title |
|
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Co-authored-by: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
this is to solve the documentation issue and close issue #50 and