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

Update doc and natspec #345

Merged
merged 7 commits into from
Dec 26, 2023
Merged

Update doc and natspec #345

merged 7 commits into from
Dec 26, 2023

Conversation

@Jean-Grimal Jean-Grimal requested review from Rubilmax, MerlinEgalite, a team, adhusson, QGarchery and MathisGD and removed request for a team December 12, 2023 14:25
Copy link
Contributor

@MerlinEgalite MerlinEgalite left a comment

Choose a reason for hiding this comment

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

Just one thing, else LGTM!

README.md Outdated Show resolved Hide resolved
Co-authored-by: Merlin Egalite <44097430+MerlinEgalite@users.noreply.github.com>
Signed-off-by: Jean-Grimal <83286814+Jean-Grimal@users.noreply.github.com>
src/MetaMorpho.sol Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Romain Milon <rmilon@gmail.com>
Signed-off-by: Jean-Grimal <83286814+Jean-Grimal@users.noreply.github.com>
/// @notice Submits a forced market removal from the vault, potentially losing all funds supplied to the market.
/// @dev Warning: Submitting a forced removal will overwrite the timestamp at which the market will be removable.
/// @notice Submits a forced market removal from the vault, eventually losing all funds supplied to the market.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not change potentially into eventually, the funds will not necessarily be lost if the timelock is reached and the market effectively removed: funds could possibly be recovered later with a reallocate if the market allows it

Copy link
Contributor

Choose a reason for hiding this comment

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

This will be addressed in #354 pls don't change this PR let's merge it

@MathisGD MathisGD merged commit e513753 into post-cantina Dec 26, 2023
6 checks passed
@MathisGD MathisGD deleted the fix/doc branch December 26, 2023 20:30
@Jean-Grimal Jean-Grimal mentioned this pull request Dec 28, 2023
@StErMi
Copy link

StErMi commented Dec 30, 2023

Looks good

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.

7 participants